Сортировка слиянием для односвязных списков дает правильные результаты, но приводит к утечке памяти

Я работаю над заданием по реализации сортировки слиянием для односвязных списков на C++. Функция merge должна объединить два отсортированных списка на месте без создания новых узлов. Функция mergesort должна создать новый отсортированный список без изменения входного списка.

Моя реализация проходит все тестовые случаи, но имеет проблему с утечкой памяти, обнаруженную Valgrind. Вот соответствующий код с примером, запущенным в main внизу:

#include <cstdio>
#include <iostream>

/* node
   The node type for our linked list.
*/
struct node {
    int value;
    node* next;

    node(int val) : value(val), next(nullptr) {}
    node(int val, node* next) : value(val), next(next) {};
};

/* merge
   Merge the lists given by `left` and `right`, returning the head of the merged
   list (the returned node should either be the head of `left` or the head of
   `right`). The two input lists will always be sorted in ascending order.

   Must run in O(m+n) time where `m` is the length of `left` and `n` is the
   length of `right`.

   Must use O(1) space (i.e., this is an in-place operation); no new nodes 
   may be created.

   NOTE: This function should modify the `next` pointers in the nodes, but NOT
   modify the `value`s. That is, it merges the lists by updating their list
   structure, not by moving values from one list to another.
*/
node* merge(node* left, node* right) {
    if (!left) return right;
    if (!right) return left;

    node* mergedHead = nullptr;
    if (left->value <= right->value) {
        mergedHead = left;
        left = left->next;
    } else {
        mergedHead = right;
        right = right->next;
    }

    node* mergedTail = mergedHead;
    while (left && right) {
        if (left->value <= right->value) {
            mergedTail->next = left;
            left = left->next;
        } else {
            mergedTail->next = right;
            right = right->next;
        }
        mergedTail = mergedTail->next;
    }

    if (left) {
        mergedTail->next = left;
    } else {
        mergedTail->next = right;
    }

    return mergedHead;
}

/* mergesort(input, length)
   Recursively Mergesort the `input` list (whose length is given by `length`),
   returning a new sorted list. 

   Must run in O(n log n) time, where n = `length`.

   Must use O(n) space (returned list is created new). 

   NOTE: The `input` list must not be modified in any way.
*/
node* mergesort(node* input, int length) {
    if (length <= 1) {
        return new node(input->value);
    }

    int mid = length / 2;
    node* left = input;
    node* right = input;
    node* prev = nullptr;

    for (int i = 0; i < mid; ++i) {
        prev = right;
        right = right->next;
    }

    prev->next = nullptr;

    node* sortedLeft = mergesort(left, mid);
    node* sortedRight = mergesort(right, length - mid);

    return merge(sortedLeft, sortedRight);
}

/* mergesort(input)
   Mergesort the list whose head is `input`, returning a new sorted list. This
   function should compute the length of the list, and then pass `input` and
   the length to the two-parameter recursive `mergesort` overload, below.

   Must run in O(n log n) time (but that's because `mergesort(node*,int)`, below
   runs in O(n log n)).

   Must use O(n) space (i.e., the returned list is created new).
*/
node* mergesort(node* input) {
    if (!input) return nullptr;

    int length = 0;
    node* temp = input;
    while (temp) {
        ++length;
        temp = temp->next;
    }

    return mergesort(input, length);
}

void printlist(node *head) {
    while (head) {
        std::cout << head->value << " ";
        head = head->next;
    }
    std::cout << "\n";
}

int main(){
    // Example input: 4->1->3->2
    node *input = new node(4, new node(1, new node(3, new node(2))));
    node *sorted = mergesort(input);
    printlist(sorted); // 1 2 3 4
    return 0;
}

Вот что сообщает Валгринд:

Сортировка слиянием для односвязных списков дает правильные результаты, но приводит к утечке памяти

Я не смог найти, где у меня происходит утечка памяти. Программа возвращает правильные результаты для тестовых случаев, но я не могу обнаружить ошибку в моей логике в отношении утечки. Что мне не хватает?

Во-первых, это не минимально воспроизводимый пример. Где находится функция main? Какие тестовые данные вы используете? Во-вторых, - но я не могу найти ошибку в своей логике - Используйте отладчик. В-третьих, я не смог найти ни одного вызова delete в том, что вы опубликовали, учитывая, что в вашем коде есть return new node(input->value);.

PaulMcKenzie 18.07.2024 10:42

valgrind сообщает, что вы выделили память в функции make_list, которая находится в файле assign3_test.cpp в строке 292. Это будет первое место, которое стоит проверить.

Yksisarvinen 18.07.2024 10:49

Утечка памяти происходит из списков ввода, размещенных вне этого кода. Это предполагает отсутствие очистки в тестовом коде. Но я не думаю, что вас удовлетворил пункт «Список input нельзя изменять каким-либо образом». требование, так что, возможно, сначала исправьте это.

teapot418 18.07.2024 10:52

@trincot, как я это читаю, функция mergesort должна создавать одну копию входных данных («Необходимо использовать пространство O(n)»), merge, с другой стороны, должна быть на месте и ничего не выделять.

teapot418 18.07.2024 11:02

Ах да, разница в функциях. Ты прав, @teapot418

trincot 18.07.2024 11:04

Значит, виноват prev->next = nullptr;. Вызывающий теряет доступ к оставшейся части входного списка и не может ее освободить.

trincot 18.07.2024 11:08

Кроме того: проверка length <= 1 неверна: если вы вызываете сортировку слиянием для списка длиной 0, у вас неопределенное поведение.

Caleth 18.07.2024 13:13

На самом деле я бы сказал, что ошибка в задании. Комментарий к коду не отражает контракт API. Данная структура не является постоянной корректной. В частности, node* input должен быть указателем на константу node. Если бы входные данные были константными, компилятор не смог бы скомпилировать, что сделало бы ошибку в коде очевидной.

Goswin von Brederlow 18.07.2024 17:33

PS: Почему нет ~node()? Возвращаемое значение должно быть std::unique_ptr<node>. Семантика копирования/перемещения также должна быть определена (по крайней мере, удалена).

Goswin von Brederlow 18.07.2024 17:44
Стоит ли изучать PHP в 2026-2027 годах?
Стоит ли изучать PHP в 2026-2027 годах?
Привет всем, сегодня я хочу высказать свои соображения по поводу вопроса, который я уже много раз получал в своем сообществе: "Стоит ли изучать PHP в...
Поведение ключевого слова "this" в стрелочной функции в сравнении с нормальной функцией
Поведение ключевого слова "this" в стрелочной функции в сравнении с нормальной функцией
В JavaScript одним из самых запутанных понятий является поведение ключевого слова "this" в стрелочной и обычной функциях.
Приемы CSS-макетирования - floats и Flexbox
Приемы CSS-макетирования - floats и Flexbox
Здравствуйте, друзья-студенты! Готовы совершенствовать свои навыки веб-дизайна? Сегодня в нашем путешествии мы рассмотрим приемы CSS-верстки - в...
Тестирование функциональных ngrx-эффектов в Angular 16 с помощью Jest
В системе управления состояниями ngrx, совместимой с Angular 16, появились функциональные эффекты. Это здорово и делает код определенно легче для...
Концепция локализации и ее применение в приложениях React ⚡️
Концепция локализации и ее применение в приложениях React ⚡️
Локализация - это процесс адаптации приложения к различным языкам и культурным требованиям. Это позволяет пользователям получить опыт, соответствующий...
Пользовательский скаляр GraphQL
Пользовательский скаляр GraphQL
Листовые узлы системы типов GraphQL называются скалярами. Достигнув скалярного типа, невозможно спуститься дальше по иерархии типов. Скалярный тип...
1
9
109
1
Перейти к ответу Данный вопрос помечен как решенный

Ответы 1

Ответ принят как подходящий

Проблема в следующем задании:

prev->next = nullptr;

Это изменяет входной список. Но в задании говорилось:

Функция mergesort должна создать новый отсортированный список без изменения входного списка.

Благодаря рекурсии эта мутация в конечном итоге изолирует каждый узел во входных данных, так что после завершения вызова mergesort верхнего уровня указатель input будет просто указывать на узел, у которого нет преемника. Как следствие, вызывающая сторона не имеет возможности освободить память, занятую любым другим узлом, принадлежащим входному списку.

Прагматичный способ решить эту проблему — отменить эту мутацию перед возвратом (но смотрите дальше). Итак, измените это:

    prev->next = nullptr;

    node* sortedLeft = mergesort(left, mid);
    node* sortedRight = mergesort(right, length - mid);

к этому:

    prev->next = nullptr;

    node* sortedLeft = mergesort(left, mid);
    node* sortedRight = mergesort(right, length - mid);

    prev->next = right; // restore!

Но поскольку ваша двухпараметрическая функция mergesort не зависит от проверки конца заданного списка на основе нулевого указателя, а только от заданной длины (и это хорошо!), вам на самом деле не нужно разбивать входной список на все, и можно просто не использовать prev.

Огромное спасибо! Вы сэкономили мне невероятное количество времени!!

Milad Khazani 18.07.2024 11:55

@MiladKhazani Вы можете это увидеть, если поставите printlist(input) после сортировки

Caleth 18.07.2024 12:48

На самом деле сделать немутирующую сортировку слиянием очень просто, вы передаете узел «один за концом» вместо длины: coliru.stacked-crooked.com/a/1496f64b7f39b52d

Caleth 18.07.2024 13:09

@trincot - удалил мои предыдущие комментарии. Я понимаю, что базовый вариант всегда будет достигнут на каждом узле, поэтому prev->next = nullptr; не нужен. Моя точка зрения заключалась в том, что должно быть быстрее, чтобы избежать сканирования левого списка на наличие mid узлов, чтобы разделить список и вместо этого иметь указатели возврата или обновления слияния и сортировки слиянием для первого и конечного узлов, начиная с базового случая, и обновляться по мере развертывания цепочки вызовов. путем передачи счетчиков для слияния.

rcgldr 20.07.2024 01:13

@trincot Ответ по-прежнему работает, спасибо. Я не уверен, почему его удалили. Я еще раз добавил согласие. Спасибо вам за помощь

Milad Khazani 22.07.2024 00:54

Другие вопросы по теме