Нужно ли вообще очищать память в этом месте?

Question

[devEngineer]

У меня есть функция которая удаляет символ из строки, путем создания новой строки и возврата указателя на эту строку. Нужно ли вообще очищать память в старом указателе?

char* erase_symbol(char* string, int index)
{
	const int size = _strlen(string) - 1;
	char* _string = (char*)malloc(sizeof(char) * size);
	for (int i = 0; i < index; i++)
	{
		*(_string + i) = *(string + i);
	}
	for (int i = index; i < size; i++)
	{
		*(_string + i) = *(string + i + 1);
	}
	free(string);  // в этой строке программа крашится
	return _string;
}

Comments

[Roman]

devEngineer, тэг С++ не в тему.

[jcmvbkbc]

Roman, а чего не в тему-то, (char*)malloc же есть?

[Roman]

jcmvbkbc,

а чего не в тему-то

erase_symbol(char* string, int index);
const char* str = "Hello, world!";
str = erase_symbol(str, index);

In C, string literals are of type char[], and can be assigned directly to a (non-const) char*. C++03 allowed it as well (but deprecated it, as literals are const in C++). C++11 no longer allows such assignments without a cast. A string literal is not necessarily a C string: if a string literal has embedded null characters, it represents an array which contains more than one string.

(char*)malloc

https://en.cppreference.com/w/cpp/memory/new/opera...

Answer 1

[Денис Загаевский]

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

Comments

[devEngineer]

Ну то есть в мейне будет такой код

// ...
char* str = "Hello, world!";
str = erase_symbol(str, index);
// ...

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

[Денис Загаевский]

devEngineer, тут нет утечки памяти, эта строка не выделяется динамически, и хранится в отдельной области памяти. Её не надо очищать. И тип у неё const char *

[devEngineer]

Денис Загаевский, Спасибо большое, теперь понятно стало почему программа крашилась.

Answer 2

[jcmvbkbc]

Нужно ли вообще очищать память в старом указателе?

Зависит от логики этой функции: если string всегда выделена в куче и нигде не остаётся ссылок на неё, то можно её освобождать. В противном случае -- нет.

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

Comments

[Александр Ананьев]

Да там все плохо. Переменная string на null не проверяется, переменная size на отрицательное значение, результат malloc тоже хорошо бы проверть на null.

[jcmvbkbc]

Александр Ананьев, да, верно, но с такими-то входными данными программа поломается быстро и в очевидном месте.

[Rsa97]

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

[devEngineer]

Rsa97, Да, я не спорю, но не дописывается ли ноль автоматически?
Есть код

// ...
char* str = "Hello, world!"; // тут ноль записывается сразу
char* new_str = erase_symbol(str, index);
// _strlen моя функция, которая считает до \0
printf("%d %d", _strlen(str), _strlen(new_str)); // вывод будет корректным
// ...

Я понимаю что я выделяю память и под ноль нужно тоже, но почему тогда моя функция работает корректно? И все-таки стоит ли явно тогда прописывать ноль при выделении памяти?

[devEngineer]

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

// ...
char* str = "Hello, world!";
str = erase_symbol(str, index);
// ...

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

ПС палками не бейте, просто ищу ответ почему так работает.

[Rsa97]

devEngineer, А кто вам сказал, что она работает корректно? Вы же не знаете, что находится в памяти за строкой. Может вам просто везёт и там ноль. А в следующий раз не повезёт, и нуля там не будет.

[jcmvbkbc]

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

devEngineer, написал же в ответе:

если string всегда выделена в куче и нигде не остаётся ссылок на неё, то можно её освобождать. В противном случае -- нет.

char* str = "Hello, world!";
-- это не в куче.

[Roman]

devEngineer, вы определитесь с языком на котором пишете, Cи и C++ на сегодняшний день это разные языки. https://en.cppreference.com/w/cpp/language/string_...
char* str = "..."; В С++ не валидно.
и это, в таком виде.

erase_symbol(char* string, int index);

char* str = "Hello, world!";
str = erase_symbol(str, index);

Кроме того у вас нет в коде ни одной идиомы языка С++ и даже функций из библиотеки языка Си включенной в стандартную библиотеку языка С++ (например <cstdlib>) в пространстве имен std. Не говоря уж о https://en.cppreference.com/w/cpp/memory/new

Если что я про тэг С++, который вы не убираете.

[Александр Ананьев]

devEngineer, почитайте литературу об операциях со строками в си. И да, как вам уже выше написали, вы пытаетесь освободить память, не выделенную в куче (т. е. через malloc) через функцию free - отсюда и падение приложения.

Answer 3

[CityCat4]

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