G
G
Gena2014-11-01 20:04:14
linux
Gena, 2014-11-01 20:04:14

Why does dynamic copying of characters leave extra memory?

Hello, I'm writing a function to escape custom characters for my own network protocol. The algorithm is painfully simple: in the loop we check each character of the string and copy it to a new string, if the character is marked as service, then first we copy the "\" character, and then the character itself.
And everything seems to be written correctly, but strange characters appear between the characters. For example, I escape two words: CONN and OK, I get this as an output .
Function code:

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
Compiler: g++
PS By the way, please tell me how to properly clear the memory in such a cycle. Thank you.

Answer the question

In order to leave comments, you need to log in

4 answer(s)
R
Rsa97, 2014-11-01
@lukinov93

Hmm ..., the efficiency of your algorithm is below the baseboard.

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;
}

And the reason for your error is simple, s is a character , and strcat concatenates strings . The difference is that the line must end with the character '\x00'. Since the memory was reserved on the stack, the symbol is located at the address (&s), and the variable i begins at the address (&s+1). For i = 1, strcat reads the string "C\x01\x00" at address &s, for i =2 - "O\x02\x00", etc.

S
SHVV, 2014-11-01
@SHVV

Some tin. Why reinvent such a wheel at all?
- firstly, your first result is not allocated to dynamic memory;
- secondly, you forget to allocate memory for terminal zero;
- thirdly, you release the memory only once, and allocate it on each character;
- fourthly, it is desirable to save memory allocation operations (ideally - to allocate only once), since they are slow and lead to fragmentation.
- strlen(string) in a loop is bad, it's better to store it in a separate variable.
I recommend deleting everything and rewriting it again.

B
bogolt, 2014-11-01
@bogolt

This has already been answered before me, but I want to add.
Author - pay attention to working with memory. If you write something like this in a real program, then such a memory leak as here can consume all the RAM on a running machine in a few minutes / hours / days, depending on the frequency of using the function.
Understand how new/delete and malloc/free work. Understand why it is impossible to allocate a variable through the new operator and then release this memory through free (even if it somehow worked for you now).
On the question - strcat takes a second argument as a pointer to a string and not a pointer to a single char, and by deceiving the compiler with the added sign & you only deceived yourself.

P
plasticmirror, 2014-11-01
@plasticmirror

Well, if you also save memory, then so

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;
}

ps does anyone understand why the author checks that '$' and ':' are not '\r' and '\n'?

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question