delete[] вызывает повреждение кучи

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

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

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

Я уже сделал что-то подобное с управлением информацией о шине, и оно отлично работало, что еще больше меня запутало... (Очевидно, я не сделал механизм точно таким же, но даже посмотрев на мой предыдущий код, я не смог выделить проблема)

Я добавил функции, которые, по моему мнению, вызывают беспокойство,

Функция добавления:

void Add_Word(char**& dictionary, int& dictionary_size, char word[])
{
    char** temp = new char*[dictionary_size + 1];   // Create a new array of appropriate size.

    int i;
    for (i = 0; i < dictionary_size; i++)
    {
        temp[i] = dictionary[i];    // Copy head pointers addresses for all existing items.
    }
    temp[i] = new char[strlen(word)];   // Add the space for the new word,
    temp[i][strlen(word)] = '\0';   // mark its end

    strcpy_s(temp[i], strlen(word) + 1, word);  // then copy it.
    // I'm really not so sure about what I should put in the buffer length but
    // strlen(word) + 1 seemed to work... I know... not good, but strlen(word) alone caused a problem.

    if (dictionary_size > 0)
        delete []dictionary;    // Delete previous head pointers array if there are any and
    dictionary = temp;  // reset the main pointer to the address of the new one.

    dictionary_size++;  // Finally, increase dictionary_size.
}

Функция удаления:

void Delete_Word(char**& dictionary, int& dictionary_size, char* word)
{
    // !!! This is where the crash thingy happens.
    delete[] Search_For_Word(dictionary, dictionary_size, word);    // Delete the word from the dictionary.
    // Search_For_Word returns a pointer to the word it receives, from the dictionary.

    char** temp = new char*[dictionary_size - 1];   // Create a new array of appropriate size.

    int i;
    for (i = 0; i < dictionary_size; i++)
    {
        if (dictionary[i][0])
            temp[i] = dictionary[i];    // Copy the head pointers of the existing
           // items to the new array except for the deleted word.
    }

    delete[] dictionary;    // Delete previous head pointers array and
    dictionary = temp;  // reset the main pointer to the address of the new one.

    dictionary_size--;  // Finally, decrease dictionary_size.
}

РЕДАКТИРОВАТЬ: Любые части, которые чрезмерно неэффективны или явно сломаны, вероятно, являются результатом того, что я возился с моим кодом, пытаясь понять это самостоятельно (например, 3 раза упомянутый вызов strlen (еще раз спасибо за это, kfsone...) , или забыл +1 для '\0', чтобы отметить конец строки - на самом деле, нет, если мы пойдем очевидным, вы не будете говорить мне о моих ошибках @.@).

Что касается причины, по которой я имею дело с char вместо строк и векторов, позвольте мне процитировать себя: "...как часть моей домашней работы". Я только начал программировать. Это, и я хочу понять основы, прежде чем перейти к использованию более удобных инструментов более высокого уровня.


person user2962533    schedule 14.01.2014    source источник
comment
AHRRG! Почему вы собираетесь работать с необработанными char указателями, чтобы реализовать это с помощью c++< /а>?? Используйте что-н. например, std::map<std::string,std::string>, пожалуйста, и забудьте о правильном управлении памятью!!   -  person πάντα ῥεῖ    schedule 15.01.2014
comment
Как насчет этого: temp[i] = new char[strlen(word)]; Должно быть strlen(word)+1; вот где ваша коррупция происходит   -  person fernando.reyes    schedule 15.01.2014
comment
Я понимаю, что вы пометили это как C++, потому что вы используете new и delete, но то, что вы делаете, это C+ или C#-, а не C++. Я предполагаю, что вы облажались, выполняя delete[] Search_For_Word ..., но ваш код выглядит по-разному ошибочным (ошибки строки off-by-1 и т. д.), вы плохо понимаете указатели и c-строки, и вы не используете какие-либо фактические грани С++. Любое из следующих двух утверждений заменяет большую часть вашего кода, устраняет ошибки и значительно повышает эффективность. std::vector<std::string>, std::vector<std::unique_ptr<std::string>>.   -  person kfsone    schedule 15.01.2014
comment
klmr.me/slides/modern-cpp.pdf   -  person pepper_chico    schedule 15.01.2014
comment
Этот код: temp[i] = new char[strlen(word)]; temp[i][strlen(word)] = '\0'; strcpy_s(temp[i], strlen(word) + 1, word); УЖАСЕН. Он вызывает strlen 3 раза (дорого), передает strlen (плюс 1! в strcpy_s (глупо, глупо, глупо), добавляет '\0' перед strcpy (все виды неправильно), существует не так уж много способов, которыми вы могли бы temp[i] = strdup(word); ошибиться еще больше.О... И индексы массива C/C++ основаны на нуле, поэтому temp[i][strlen(word)] = '0' всегда записывает в один элемент после конца распределения.   -  person kfsone    schedule 15.01.2014
comment
@kfsone Я ценю обратную связь, я просто хочу, чтобы вы передали ее менее оскорбительным тоном...   -  person user2962533    schedule 15.01.2014


Ответы (4)


Изменять:

temp[i] = new char[strlen(word)]

To:

temp[i] = new char[strlen(word)+1]
person barak manos    schedule 14.01.2014
comment
Исправлено, но проблема осталась. Я также: temp[i][strlen(word) + 1] = '\0'; хотя, если я правильно понял kfsone, '\0' все равно пойдет в последний слот. - person user2962533; 15.01.2014
comment
Должно быть temp[i][strlen(word)] = '\0' (без +1) - person barak manos; 15.01.2014

В вашем коде есть несколько проблем.

Во-первых, если вы хотите выделить строку в стиле C в куче с помощью new[], вам следует обратить внимание на завершающий символ NUL.

Таким образом, если вы хотите сделать глубокую копию строки word, вы должны рассчитать достаточно места, учитывая strlen(word) + 1: +1 предназначен для завершающего символа NUL.
например:

// Original code (wrong):
//
//     temp[i] = new char[strlen(word)];
//
// New code:
temp[i] = new char[strlen(word) + 1]; // consider terminating NUL (+1)

Более того, следовать вашему коду с явными new[] и delete[] непросто.
В современном C++ вы можете захотеть использовать удобные надежные контейнерные классы, такие как std::vector, и строковые классы, такие как std::string, вместо необработанного C- указатели стилей и строки.

Вы можете просто сохранить список строк, используя методы std::vector<std::string> и vector::push_back(), чтобы добавить новые строки в вектор. Не нужно усложнять код new[], delete[], strcpy_s() и т.д.

И если вы хотите глубоко скопировать строки, вы можете просто использовать простую естественную перегрузку operator= для std::string и копировать конструкторы; например std::string temp = word; будет работать нормально.

person Mr.C64    schedule 14.01.2014

Это C++, почему вы не используете std::string вместо char буферов?

Если вы должны использовать строки буфера char и безопасные формы strcpy_s, знайте, что длина буфера всегда должна быть размером целевого буфера, а не функцией strlen. В вашем случае это немного понятно, поскольку вы создали буфер с помощью функции strlen. Но что вы должны сделать, так это установить значение в переменную, а затем использовать ее в любое время, когда вам нужен размер буфера.

Кроме того, и где я думаю, что ваша ошибка, вы пишете temp[i][strlen(word)] = '\0'; Но фактические индексы буфера идут от 0 до strlen(word)-1, поэтому вы пишете за пределами выделенной памяти.

person Zan Lynx    schedule 14.01.2014

Теперь код работает.

Это было неправильно во всем. Я испортил почти все, что мог, относительно динамической памяти, пытаясь исправить это раньше.

Сначала я не хотел звонить 3 раза в strlen, потому что это просто домашняя работа и очень маленькая программа, но я думаю, что лучше привыкнуть делать все правильно... Я также сбросил копию, которую, очевидно, не понимаю очень хорошо в пользу простого цикла for.

// Add function. The rest is cut.
    int word_length = strlen(word);

    temp[i] = new char[word_length + 1];    // Added +1 here.
    temp[i][word_length] = '\0';    /* This was correct after all.
    the word_length index is the correct ending.*/

    for (int j = 0; j < word_length; j++)   // copy replaced by for loop.
        temp[i][j] = word[j];
    // cut
}

void Delete_Word(char**& dictionary, int& dictionary_size, char* word)
{
    delete[] Search_For_Word(dictionary, dictionary_size, word);
   // There was a -1 mistake here I made in order to try and fix the thing earlier.
// No need for more, it works perfectly now.
person user2962533    schedule 15.01.2014