Почему при динамическом копировании символов, остается лишняя память?

Question

[Гена]

Здрасьте, пишу функцию для экранирования кастомных символов для собственного сетевого протокола. Алгоритм, до боли прост: в цикле проверяем каждый символ строки и копируем его в новую строку, если символ помечен как служебный, то сначала копируем символ " \", а затем сам символ.
И вроде бы все написано правильно, но между символами появляются странные символы. К примеру, я экранирую два слова: CONN и OK, на выходе получаю это.

Код функции:

char *result = "", *temp, s;
for(int i = 0; i < strlen(string); i++) {
	s = string[i];
	temp = result;
	// Если найден спец. символ
	if((s == ':' || s == '$') && s != '\r' && s != '\n') {
		// Выделяем доп. память и копируем туда два символа
		result = new char[strlen(temp) + 2];
		strcpy(result, temp);
		strcat(result, "\\");						// Первый символ
		strcat(result, &s);						// Второй символ
	} else {
		// Копируем текущий символ без изменений
		result = new char[strlen(temp)];
		strcpy(result, temp);
		strcat(result, &s);
	}
}
free(temp);
printf("%s\n", result);
return result;

OS: Ubuntu 14.01
IDE: Eclipse Luna
Компилятор: g++

P.S. Кстати подскажите пожалуйста как правильно очистить память в таком цикле. Спасибо.

Answer 1

[Rsa97]

Мда..., эффективность вашего алгоритма ниже плинтуса.

char *escapeString(char *str) {
    char *result, *temp;
    int i;
    temp = malloc(strlen(str)*2+1);
    i = 0;
    while (*str) {
        switch(*str) {
            case '\n':
                temp[i++] = '\\';
                temp[i++] = 'n';
                break;
            case '\r':
                temp[i++] = '\\';
                temp[i++] = 'r';
                break;
            case ':':
            case '$':
            case '\\':
                temp[i++] = '\\';
            default:
                temp[i++] = *str;
        }
        str++;
    }
    temp[i] = 0;
    result = malloc(i+1);
    strcpy(result, temp);
    free(temp);
    return result;
}

А причина ошибки у вас простая, s - символ, а strcat объединяет строки. Разница в том, что в конце строки обязан стоять символ '\x00'. Поскольку память резервировалась в стеке, то по адресу (&s) находится символ, а по адресу (&s+1) - начинается переменная i. При i = 1 strcat по адресу &s читает строку "C\x01\x00", при i =2 - "O\x02\x00" и т.д.

Comments

[Гена]

Спасибо за ответ, учту все корректировки

Answer 2

[SHVV]

Жесть какая-то. Зачем вообще изобретать такой велосипед?
- во-первых, первый result у вас не выделен на динамической памяти;
- во-вторых, забываете выделить память под терминальный ноль;
- в-третьих, память освобождаете только один раз, а выделяете на каждом символе;
- в-четвёртых, операции выделения памяти желательно экономить (идеально - выделять только один раз), так как они медленные и ведут к фрагментации.
- strlen(string) в цикле - плохо, лучше сохранить в отдельную переменную.

Рекомендую удалить всё и переписать заново.

Comments

[Гена]

Ну насчет памяти я знаю, поэтому и попросил совета по ее очистке. На счет терминального нуля я не знал. Спасибо.

Answer 3

[bogolt]

До меня тут уже ответили, но хочу добавить.
Автор - обратите внимание на работу с памятью. Если вы напишите подобное в реальной программе, то такая утечка памяти как здесь сможет за несколько минут/часов/дней в зависимости от частоты использования функции выжрать всю оперативку на запущенной машине.
Разберитесь как работают new/delete и malloc/free. Разберитесь почему нельзя выделить переменную через оператор new и потом освобождать эту память через free ( даже если сейчас у вас это каким-то образом сработало ).
По вопросу - strcat принимает вторым аргументом указатель на строку а не указатель на единственный char и обманув компилятор добавленным значком & вы обманули лишь себя.

Answer 4

[plasticmirror]

ну, если еще и память экономить - то так

char * escape(char* input)
{
	int resultLength = 0;
	char* p = input;

	while(*p) {
		if(*p == ':' || *p == '$')
			resultLength++;
		resultLength++;
		p++;
	}
	
	char * result = (char*) malloc(resultLength+1);
	int j = 0;
	p = input;
	
	do {
		if(*p == ':' || *p == '$')
			result[j++] = '\\';
		result[j++] = *p;
	} while(*(p++));
	
	return result;
}

п.с. кто-нибудь понял, зачем автор проверяет что '$' и ':' не являются '\r' и '\n'?

Comments

[Гена]

Символы переноса строк не должны попадать в результат, благодарю за помощь.