Как правильно чистить память в c++?

Question

[Кирилл Журавлев]

Всем спасибо за комментарии. Принял решение переписать свой колхоз на векторах, чтобы не заморачиваться с выделением и очисткой памяти.

Пишу простенький класс на C++.
В классе есть такой конструктор:

Polynomial::Polynomial(int stepen)
{
	deg = stepen;
	data = new int[deg + 1];
	for (int i = deg; i >= 0; i--) data[i] = 0;
}

И такой деструктор:

Polynomial::~Polynomial()
{
	delete[] data;
}

Однако по завершению программы вылетает исключение с таким содержанием:
HEAP CORRUPTION DETECTED

Еще есть такой кусок кода, валится на выделенной строке:

int maxdeg = 0;
	for (int i = 0; i <= deg; i++) if (data[i] != 0) maxdeg = i;
	int *newdata = new int[maxdeg+1];
	for (int i = 0; i <= deg; i++) newdata[i] = data[i];
	deg = maxdeg;
	int *pointersaver = data;
	data = newdata;
	delete[] pointersaver;  //Тут

Самое забавное, что такая проблема только тогда, когда я собираю под Debug. Под Release программа отрабатывает корректно до конца.

Что я делаю не так?

Answer 1

[res2001]

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

Ошибка тут:
for (int i = 0; i <= deg; i++) newdata[i] = data[i];
Вместо deg нужно использовать maxdeg, имхо.

Кроме того вы не проверяете значение deg и maxdeg до выделения памяти. Что если они содержат значения <= 0?

Возможно, есть и другие места с присваиванием массиву data и переменной deg. Нужно искать выход за границу массива при присваивании элементам массива из-за чего повреждается куча.
Так же настораживает new int[deg +1] и операторы >= и <= в условиях циклов обхода массива, это потенциальные места выхода за границу. Возможно, если перейти от deg к количеству элементов в массиве, то код упростится (уберете +1 при выделении и = в условии циклов) и будет легче найти ошибку.

PS: в качестве небольшой оптимизации: для обнуления массива можно использовать функцию memset, для копирования массивов - memcpy.

Comments

[Евгений Шатунов]

res2001 , добавлю тебе в ответ мои пять копеек.
В отладочной сборке не "скорее всего", а точно включается код проверки целостности кучи.

Если иметь окружение MS Visual Studio, то при отладке даже сама куча работает в режиме отладки. Это уже означает, что при выделении памяти происходит маркировка ее границ - установка т.н. забора, а в заголовке выделенного участка пишутся дополнительные данные.
Сам забор из себя представляет 4 одинаковых байта перед выделенным участком (и после его заголовка) и 4 таких же одинаковых байта сразу после выделенного участка.
При освобождении выделенной памяти делается гора проверок, среди которых - проверка целостности забора. Это - одна из основных причин падения производительности при запуске в отладке в MS VC++.

Кирилл Журавлев , у тебя код явно забор ломает. Тебе стоит проверить, соблюдаются ли у тебя диапазоны в циклах и правильные ли индексы используются при записи в твой массив.
Я в твой код въезжать не планирую, поэтому ответов не даю. Просто проверь индексы.
К тому же, res2001 все качественно описал.

Answer 2

[Therapyx]

Когда речь идет о выделении памяти, т.е. создании динамических массивов или просто указателей на блоки памяти. То выглядит это следующим образом:
int size = ...
int *array = new int[size];

или

int* p = NULL;
p = new int;

замень, тут всегда идет указатель на выделенный блок памяти.
Соотвественно и деструкторе для массивов идет
delete [] array;
или delete p;

В твоем примере

Polynomial::Polynomial(int stepen)
{
  deg = stepen;
  data = new int[deg + 1];
  for (int i = deg; i >= 0; i--) data[i] = 0;
}

Polynomial::~Polynomial()
{
  delete[] data;
}

все вроде бы верно, при условии, что дата это указатель, который декларирован где-то в классе заранее.

Answer 3

[Элль Соломина]

Точный ответ на вопрос дать не могу поскольку код представлен не полностью. Вероятнее всего data в классе объявлена не верно, она должна быть указателем на массив и никак иначе, pointersaver в коде ниже тоже должен быть указателем на массив, хотя бы преобразовываться к этому типу (с помощью static_cast) в месте вызова delete.

Вообще при использовании C++ настоятельно рекомендуется использовать ссылки, умные указатели unique_ptr, и готовые конструкции для типов, подойдёт array.

Если же целью ставится разобраться в "сырых" указателях и работе с ними, то стоило бы вместо цикла for (int i = deg; i >= 0; i--) data[i] = 0; использовать memset.

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

P.S. в Release код сильно изменяется компилятором в процессе оптимизации и из-за большего количества "знаний о коде" компилятор может сгенерировать его гораздо чище чем он был написан, а в Debug оптимизации не применяются и поэтому код будет работать так как написан.

Доп: а вообще вот нашёл хорошую статью по теме https://habrahabr.ru/company/abbyy/blog/117208/ там рассказано как new[] delete[] работает на примерах с ошибками.

Answer 4

[asd111]

Используй valgrind и спирать pvs studio или хотя бы cppcheck.

Answer 5

[Xilian]

Я думаю при релизе просто первая и третья строка будут отброшены компилятором, как неиспользуемые. А так ты же память для pointersaver не выделял - у тебя там случайный указатель, вот delete[] отлуп и схватывает.

int *pointersaver = data;
data = newdata;
delete[] pointersaver; //Тут

Answer 6

[Лежебокер]

Вроде не надо скобки у delete ставить.

Comments

[Кирилл Журавлев]

Вроде как динамически выделенную память так чистить надо.

[Лежебокер]

Кирилл Журавлев, ну я даже не знаю. В моей книжонке да и впринципе на практике я нормально пользовался без этого

[Евгений Шатунов]

Лежебокер , очень советую ознакомиться с документацией на операторы new и delete.
Очень важно понимать, что в общем случае связка "new[] - delete" - это Undefined Behavior.

Безусловно, реализация delete[] обычно сводится к циклу финализации и вызову простого delete. Но это не значит что этим можно пользоваться. Одна маленькая правка и у тебя в коде UB, приехали.
Лучше будет положиться на оптимизацию компилятора, он все сделает как надо.

Answer 7

[Antonio Solo]

у тебя выход за границы массива

int *newdata = new int[maxdeg+1];
for (int i = 0; i <= deg; i++) newdata[i] = data[i];