Nano Hash - криптовалюты, майнинг, программирование

Почему эта функция вызывает утечку памяти?

Я написал свою собственную структуру данных (связанный список) и использовал ее в приведенном ниже коде. Когда я анализирую программу с помощью valgrind, методы push и push_back связанного списка вызывают утечку памяти. Не могли бы вы помочь мне найти, почему это происходит?

Связанный список:

template <typename T>
struct Node {
  T data;
  Node *next;
};

/**
 * @brief Simple Linked List implementation
 * 
 * @tparam T 
 */
template <typename T> class List{
private:

public:

    Node<T> *head;


    /**
     * @brief Amount of nodes in the list
     * 
     */
    int length;
    /**
     * @brief Construct a new List object
     * 
     */
    List(){
        head = NULL;
        length = 0;
    }

    /**
     * @brief Add new node to the list and increase size
     * 
     * @param val 
     */
    void push(T val){
        Node<T> *n = new Node<T>();   
        n->data = val;             
        n->next = head;        
        head = n;              
        length++;
    }

    /**
     * @brief Add new node to the end of the list and increase size
     * 
     * @param val 
     */
    void push_back(T val) {
        Node<T> *n = new Node<T>();
        Node<T> * temp = head;
        n->data = val;
        n->next = nullptr;
        if (head) {
            while (temp->next != NULL) {
                temp = temp->next;
            }
            temp->next = n;
        } else {
            head = n;
        }
        length++;
    }

    /**
     * @brief Remove the node from the list and decrease size
     * 
     * @return T 
     */
    T pop(){
      if(head) {
        T p = head->data;
        head = head->next;
        length--;
        return p;
      }
    }


/**
 * @brief Get n-th item on the list
 * 
 * @param index Index of the item 
 * @return T 
 */
    T get(int index) {
        T value_to_return;
        Node<T> * temp = head;
        if (index == 0) {
            return head->data;
        }
        for (int i = 0; i < index; i++) {
            temp = temp->next;
            value_to_return = temp->data;
        }
        return value_to_return;
    }
};

Код, вызывающий ошибки:

/**
 * @file file_reader.h
 * @author Dawid Cyron ([email protected])
 * @brief File with functions used for processing required text files
 * @version 0.1
 * @date 2020-01-26
 * 
 * @copyright Copyright (c) 2020
 * 
 */
#include <vector>
#include "bibliography.h"
#include <fstream>
#include <regex>
#include "list.h"
#include "map.h"


/**
 * @brief Function used for sorting list of bibliography
 * 
 * @param bibliography_list List to sort
 */
void sort_bibliography_list(List<bibliography> bibliography_list) {
    Node <bibliography> * current = bibliography_list.head, * index = NULL;
    bibliography temp;

    if (bibliography_list.head == NULL) {
        return;
    } else {
        while (current != NULL) {
            index = current->next;

            while (index != NULL) {
                if (current->data.author.substr(current->data.author.find(" "), current->data.author.length() - 1) > index->data.author.substr(index->data.author.find(" "), index->data.author.length() - 1)) {
                    temp = current->data;
                    current->data = index->data;
                    index->data = temp;
                }
                index = index->next;
            }
            current = current->next;
        }   
    }
}

/**
 * @brief Funciton used for reading the contents of bibliography file
 * 
 * @param filename Name of the file containing bibliography
 * @return std::vector < bibliography > Vector containing bibliography objects (tag, author, book title), alphabetically sorted by surname
 */
List < bibliography > readBibliographyFile(char * filename) {
    std::ifstream bibliography_file(filename);
    std::string line;
    int line_counter = 0;
    bibliography bib;

    List<bibliography> storage_test;

    if (bibliography_file.is_open()) {
        while (getline(bibliography_file, line)) {
            if (line_counter == 0) {
                if (line == "") {
                    std::cout << "Incorrect data format. Exiting" << std::endl;
                    exit(1);
                }
                bib.label = line;
            } else if (line_counter == 1) {
                if (line == "") {
                    std::cout << "Incorrect data format. Exiting" << std::endl;
                    exit(1);
                }
                bib.author = line;
            } else if (line_counter == 2) {
                if (line == "") {
                    std::cout << "Incorrect data format. Exiting" << std::endl;
                    exit(1);
                }
                bib.book = line;
                storage_test.push_back(bib);
                line_counter = 0;
                // Skip the empty line
                getline(bibliography_file, line);
                continue;
            }
            line_counter++;
        }
    }
    sort_bibliography_list(storage_test);
    return storage_test;
}


/**
 * @brief Function used to load references footer
 * 
 * @param references List of references
 * @param output Reference to the output file
 */
void loadReferenceFooter(List<std::string> references, std::ofstream & output) {
    output << "\nReferences\n \n";;
    for (int i = 0; i < references.length; i++) {
        output << references.get(i);
    }
}

/**
 * @brief Function used to replace cite tags with referenes
 * 
 * @param filename Name of the file containing the text
 * @param data Vector of Bibliography objects (tag, author, book title), has to be sorted by surname
 * @param output_filename Name of the file where the content should be saved
 */
void replaceCites(char * filename, List < bibliography > data, char * output_filename) {
    std::ifstream text_file(filename);
    std::string content;
    content.assign((std::istreambuf_iterator < char > (text_file)), (std::istreambuf_iterator < char > ()));
    //std::map < std::string, int > map;
    std::ofstream output(output_filename);
    int cite_counter = 1;
    List<std::string> references;
    Hashtable<std::string, int> hash_table; 
    HashtableItem<std::string, int> * item;

    for (int i=0; i < data.length; i++) {
        std::smatch matches;
        std::regex regex("\\\\cite\\{" + data.get(i).label + "\\}");
        std::regex_search(content, matches, regex);
        if (!matches.empty()) {
            item = hash_table[data.get(i).label];
            if (item != nullptr) {
                content = std::regex_replace(content, regex, "[" + std::to_string(item->Value()) + "]");
            } else {
                content = std::regex_replace(content, regex, "[" + std::to_string(cite_counter) + "]");
                references.push_back("[" + std::to_string(cite_counter) + "] " + data.get(i).author + ", " + data.get(i).book + "\n");
                hash_table.Add(data.get(i).label, cite_counter);
                cite_counter++;
            }
        }
    }
    output << content << std::endl;
    text_file.close();
    loadReferenceFooter(references, output);
    output.close();
}

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


  • Я попытался создать деструктор, который прошелся по всем узлам связанного списка и удалил их один за другим Что происходит с теми, которые удаляются с помощью pop. Кроме того, если вам нужна помощь в исправлении кода удаления, вам нужно показать его. 04.02.2020
  • В чем смысл функции push()? Он создает круговой список с одним узлом, который несовместим с остальной частью вашего класса. 04.02.2020
  • pop() тоже сломан. Деструктора вообще нет? Весь класс теряет память, это не относится к push() или push_back(). 04.02.2020
  • Да, ребята, я понимаю, что pop() дает утечку, но я не использую этот метод. Я просто забыл его удалить, так как оставляю только те части, которые мне нужны. То же самое касается push(), я использовал его раньше. 04.02.2020
  • минимально воспроизводимый пример — отличная штука. Сокращая код проблемы до тех пор, пока не останется только ошибка, обычно очень легко увидеть, в чем заключается ошибка. 04.02.2020
  • Традиционно доступ к связанным спискам осуществляется без номеров индексов. Функция get должна быть поиском по значению, а затем вы должны учитывать несуществующее значение, обычно выбрасывая исключение. Вы можете попробовать пофантазировать с std::optional. 04.02.2020
  • Я понимаю, что мог бы пофантазировать насчет этого, мне, вероятно, нужно было бы больше обрабатывать ошибок и все такое, но это всего лишь быстрый проект для универа. Если бы это был мой личный проект, я бы, наверное, уделил ему гораздо больше внимания, но с этим у меня сжатые сроки и много подготовки к экзаменам. 04.02.2020
  • push_back должен анализировать весь список каждый раз. не замужем. время. вы хотите, чтобы push_back был таким медленным. Просто сохраните хвостовой указатель. 04.02.2020
  • Не фантазируй. Код почти всегда лучше, если вы упрощаете его. Если удаление узлов вызвало проблемы, вы должны были это отладить. Если вам посчастливилось получить надежный и повторяемый сбой, исправьте сбой. Всегда выбирайте для отладки решение, которое должно работать, а не другое решение, которое не может работать. 04.02.2020
  • Если ваша домашняя работа не состоит в том, чтобы написать связанный список, почему бы просто не использовать std::list и покончить с этим? То же самое касается карты. 04.02.2020
  • @DawidCyron Мне, вероятно, следует больше обрабатывать ошибки и все такое, но это всего лишь быстрый проект для университета - какое грустное отношение. По моему мнению, хороший разработчик заботится о корректности и качестве каждого проекта; одноразовый, прототип, производство - не имеет значения, вы должны всегда стремиться делать это правильно, чтобы это стало привычкой и способом по умолчанию. Срезание углов является причиной невообразимого количества программных ошибок. И если вы небрежны в мелочах, вы, вероятно, будете небрежны и в больших вещах. 04.02.2020
  • Нужно следить за правилом трех. Если у вас есть класс, который владеет ресурсом, вам необходимо убедиться, он скопирован правильно или отключите копирование, потому что вы никогда не знаете, когда вы можете случайно сделать копию, а затем взорвать позже. 04.02.2020
  • Повторяю некоторые моменты @JesperJuhl, но не из-за гордости за продукт. Мне лень. НАМНОГО проще отлаживать программу, которая проверяет и сообщает вам, когда что-то не так. 04.02.2020

Ответы:


1

Утечка памяти, потому что нет деструктора для освобождения выделенных Nodes. Спрашивающий отмечает, что они удалили деструктор, потому что программа вылетала, когда он у них был. Это ошибка, которую нужно было исправить, потому что наличие деструктора — правильное решение.

Решение

Верните деструктор обратно и решите, почему деструктор вызвал сбой программы.

Решение для решения

List нарушает правило трех. Это означает, что когда копируется List, и у вас есть два объекта, указывающие на одну и ту же голову Node. Этот Node может быть deleted только один раз, и оба объекта будут пытаться delete его List деструктором. Рано или поздно программа умирает мучительной смертью.

Обычно вы можете заменить проходы по значению проходами по ссылке, а затем запретить копирование с помощью deleteing специальных функций-членов. Например. добавлять

List(const List & src) = delete;
List& operator=(List src) = delete;

на List и подождите, пока компилятор начнет кричать о копировании List при удалении специальных функций копирования. Замените все проходы по значению проходами по ссылке.

К сожалению

List<bibliography> readBibliographyFile(char * filename)

возвращается по значению. Возврат по ссылке локальной переменной обречен. Локальная переменная выходит за пределы области видимости и уничтожается, оставляя вам ссылку на недопустимый объект. Это означает, что вы должны сделать это трудным путем:

Реализуйте все три специальные функции-члены:

// destructor
~List()
{
    while (head)
    {
        Node<T> * temp = head->next;
        delete head;
        head = temp;
    }
}

// copy constructor
List(const List & src): head(NULL), length(src.length)
{
    Node<T> ** destpp = &head; // point at the head.
                               // using a pointer to a pointer because it hides 
                               // the single difference between head and a Node's 
                               // next member: their name. This means we don't need 
                               // any special cases for handling the head. It's just 
                               // another pointer to a Node.
    Node<T> * srcnodep = src.head;
    while (srcnodep) //  while there is a next node in the source list
    {
        *destpp = new Node<T>{srcnodep->data, NULL}; // copy the node and store it at 
                                                     // destination
        destpp = &((*destpp)->next); // advance destination to new node
        srcnodep = srcnodep->next; // advance to next node in source list
    }
}

List& operator=(List src) // pass list by value. It will be copied
{
    length = src.length; // Technically we should swap this, but the copy 
                         // is gonna DIE real soon.
    // swap the node lists. use std::swap if you can.
    Node<T> * temp = head;
    head = src.head; 
    src.head = temp;

    // now this list has the copy's Node list and the copy can go out of scope 
    // and destroy the list that was in this List.

    return *this;
}

Заметки

operator= использует преимущества идиомы копирования и замены. Часто это не самое быстрое решение, но его легко написать и почти невозможно ошибиться. Я начинаю с копирования и замены и перехожу на что-то более быстрое только тогда, когда профилирование производительности кода показывает, что мне нужно, а этого почти никогда не происходит.

Трюк с указателем, используемый в конструкторе копирования, также очень удобен при вставке и удаление элементов списка.

Знайте и понимайте Правило трех и его его друзей. Без него вы не сможете писать сложные и эффективные программы на C++. Возможно, это задание было дано, по крайней мере частично, для того, чтобы заставить вас выучить его.

04.02.2020

2

«Почему эта функция вызывает утечку памяти?» - Просто: вы выделяете память с помощью new, которую вы никогда не освобождаете с помощью delete.

В современном C++ обычно лучше использовать интеллектуальные указатели (в данном случае std::unique_ptr) и/или классы контейнеров, а не ручное управление памятью с помощью new/delete.

04.02.2020
  • Не могли бы вы показать мне, как мне реализовать интеллектуальные указатели в этом случае? Я пробовал, но не мог понять, как это сделать. У меня также есть крайний срок, установленный на очень короткое время, поэтому любая помощь приветствуется. 04.02.2020
  • @DawidCyron 1) ваши временные ограничения не имеют значения, а не наша проблема. 2) эти ссылки могут помочь: en.cppreference.com/w/cpp/memory/unique_ptr , en.cppreference.com/w/cpp/memory/unique_ptr /make_unique — там есть примеры. 04.02.2020
  • презентация Херба Саттера об умных указателях, сделанная несколько лет назад. Он просматривает связанный список умного указателя примерно через 20 минут. 04.02.2020

  • 3

    Если вы можете выбрать, какой тип списка вы реализуете, вот векторная версия (по сравнению с вашим связанным списком)

    #include <cstdlib>
    #include <iostream>
    
    template <typename T>
    struct list final {
      T* values;
      std::size_t capacity, size;
    
      list() : values{nullptr}, capacity{0u}, size{0u} {}
      ~list() { std::free(values); }
      void push_back(T value) {
        if (size == capacity) {
          capacity = (capacity + 1) * 2;
          if (void* mem = std::realloc(values, capacity * sizeof(T))) {
            values = reinterpret_cast<T*>(mem);
          } else {
            throw std::bad_alloc();
          }
        }
        values[size++] = value;
      }
    
      void pop_back() { --size; }
    };
    
    int main() {
      list<int> integers;
      for (int i = 0; i < 10; ++i) {
        integers.push_back(i);
      }
      for (int i = 0; i < integers.size; ++i) {
        std::cout << integers.values[i] << std::endl;
      }
    }
    
    04.02.2020

    4

    Хорошо, я узнал, в чем проблема для меня, и я сделал довольно грязное исправление, но оно работает. Для тех, кто сказал мне, что нужно больше заботиться о коде, я улучшу его, как только закончу свои экзамены, но с таким строгим сроком мне просто нужно самое быстрое решение. Вы можете найти его ниже.

    Я добавил эту функцию в список:

    void clear() {
        while (head) {
            Node<T> * temp = head->next;
            delete head;
            head = temp;
            length--;
        }
    }
    

    И затем в конце функции replaceCites я вызываю:

    data.clear();
    references.clear();
    
    04.02.2020
  • Несвязанный: это только скрывает нарушение правила трех. Если этот код работает, значит, вам не повезло, и обстоятельства скрывают ошибку. В void replaceCites(char * filename, List < bibliography > data, char * output_filename) List < bibliography > data передается по значению, в результате чего используемый аргумент копируется в data. В List` отсутствует конструктор копирования, поэтому компилятор генерирует конструктор, который просто копирует указатель, и теперь у вас есть два List, указывающих на одни и те же данные. Это взорвалось, когда у вас был деструктор, потому что и копия, и оригинал пытались удалить узлы. 04.02.2020
  • Если ваш преподаватель действительно знает C++ и обращает внимание, он уловит эту ошибку и сократит ваши оценки. Если они этого не улавливают, молитесь, чтобы они не обращали внимания, поскольку альтернативой является то, что вас учит некомпетентный человек, и вы будете страдать из-за этого позже. 04.02.2020
  • Новые материалы

    Кластеризация: более глубокий взгляд
    Кластеризация — это метод обучения без учителя, в котором мы пытаемся найти группы в наборе данных на основе некоторых известных или неизвестных свойств, которые могут существовать. Независимо от..

    Как написать эффективное резюме
    Предложения по дизайну и макету, чтобы представить себя профессионально Вам не позвонили на собеседование после того, как вы несколько раз подали заявку на работу своей мечты? У вас может..

    Частный метод Python: улучшение инкапсуляции и безопасности
    Введение Python — универсальный и мощный язык программирования, известный своей простотой и удобством использования. Одной из ключевых особенностей, отличающих Python от других языков, является..

    Как я автоматизирую тестирование с помощью Jest
    Шутка для победы, когда дело касается автоматизации тестирования Одной очень важной частью разработки программного обеспечения является автоматизация тестирования, поскольку она создает..

    Работа с векторными символическими архитектурами, часть 4 (искусственный интеллект)
    Hyperseed: неконтролируемое обучение с векторными символическими архитектурами (arXiv) Автор: Евгений Осипов , Сачин Кахавала , Диланта Хапутантри , Тимал Кемпития , Дасвин Де Сильва ,..

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

    Обеспечение масштабируемости LLM: облачный анализ с помощью AWS Fargate и Copilot
    В динамичной области искусственного интеллекта все большее распространение получают модели больших языков (LLM). Они жизненно важны для различных приложений, таких как интеллектуальные..