Can't free an allocated string memory after appending character

Question

I am trying to append a character to a string... that works fine unfortunately I can't free the mem of the string afterwards which causes that the string gets longer and longer.... as it reads a file every linie will be added to the string which obviously shouldn't happen

char* append_char(char* string, char character)
{
        int length = strlen(string);
        string[length] = character;
        string[length+1] = '\0';
        return string;
}

I allocated mem for string like

char *read_string = (char *)malloc(sizeof(char)*500);

call the function append_char(read_string,buffer[0]); and free it after the whole string is build free(read_string);

I presume that once I call the append_char() , the mem allocation is going to be changed, which cause that I can't get hold of it.

Edited: here is the function which uses the append_char()

char *read_log_file_row(char *result,int t)
{
filepath ="//home/,,,,,/mmm.txt";
int max = sizeof(char)*2;
char buffer[max];
char *return_fgets;

char *read_string = malloc(sizeof(char)*500);

file_pointer = fopen(filepath,"r");

if(file_pointer == NULL)
{
    printf("Didn't work....");
    return NULL;
}   

int i = 0;

while(i<=t)
{
  while(return_fgets = (fgets(buffer, max, file_pointer)))
  {
    if(buffer[0] == '\n') 
    {
       ++i;
       break;   
    }   

        if(i==t)
    {
      append_char(read_string,buffer[0]);
     }      
   }

   if(return_fgets == NULL)
   {
      free(read_string);
      return NULL;                              
/*              return "\0";*/
        }
       if(buffer[0] != '\n') 
        append_char(read_string,buffer[0]);

   }    
   fclose(file_pointer);
    strcpy(result,read_string); 
    free(read_string);
   return result;
}

Answer 1

1. Dont cast the return value of malloc() in C.
2. Make sure you initialize read_string to an empty string before you try to append to it, by setting read_string[0] = '\0';.
3. Make sure you track the current length, so you don't try to build a string that won't fit in the buffer. 500 chars allocated means max string length is 499 characters.

Not sure what you expect should happen when you do free(read_string). It sounds (from your comment to @Steve Jessop's answer) that you do something like this:

char *read_string = malloc(500);
read_string[0] = '\0';  /* Let's assume you do this. */
append_char(read_string, 'a'); /* Or whatever, many of these. */
free(read_string);
print("%c\n", *read_string); /* This invokes UNDEFINED BEHAVIOR. */

This might print an a, but that proves nothing since by doing this (accessing memory that has been free():d) your program is invoking undefined behavior, which means that anything could happen. You cannot draw conclusions from this, since the "test" is not valid. You can't free memory and then access it. If you do it, and get some "reasonable"/"correct" result, you still cannot say that the free():ing "didn't work".

Answer 2

No, the memory allocation is not changed in any way by append_char. All it does is change the contents of the allocation -- by moving the nul terminator one byte along, you now care about the contents of one more of your 500 bytes than you did before.

If the string gets longer than 500 bytes (including terminator), then you have undefined behavior. If you call strlen on something that isn't a nul-terminated string, for example if you pass it a pointer to uninitialized memory straight from malloc, then you have undefined behavior.

Undefined behavior is bad[*]: feel free to read up on it, but "X has undefined behavior" is in effect a way of saying "you must not do X".

[*] To be precise: it's not guaranteed not to be bad...

Answer 3

Have you ever initialized the string? Try *read_string=0 after allocating it. Or use calloc. Also, have your string grown beyond the allocated memory?