malloc() reusing memory location after free(), causing fragmentation(?)

Question

All right, I've given this a lot of effort, looked through a lot of questions here, but I can't quite get to the bottom of this. Hopefully you can shed a little light on my problem here!

int main(int argc, char *argv[]){

  char read[50]; 
  char *string[10];

  while(1){

    fgets(read,sizeof read, stdin);

    int t = 0; //ticks whenever a non-whitespace char is read 
    int pos = 0; //keeps track of position in string array 
    int i;
    int j;

    for(i = 0; i < sizeof read; i++){
      if(isspace(read[i]) && t != 0){
        string[pos] = malloc((t+1) * (sizeof(char)));
//      int z;
//      for(z = 0; z < sizeof string[pos]; z++){
//        string[pos][z] = '\0';
//      }
        for(j = 0; j < t; j++){
          string[pos][j] = read[i-(t-j)];
        }
        t = 0;
        pos++;
      }
      else if(!isspace(input[i])) t++;
    }

    int k;
    for(k = 0; k < pos; k++){
      printf("%i: %s\n",k,string[k]);
      free(string[k]);
    }
  }
}

I am attempting to write a program which will read in a sentence from the user, which is then broken up into its constituent words, each stored in its own char array.

I use malloc() to allocate enough memory to fit each word, freeing it after use.

The first run is fine, but upon subsequent loops words shorter than 5 characters (and only if more than one word is typed in, separated by a space) will not display properly, with random extra characters/symbols appended to them.

Is this perhaps due to malloc using freed memory which isn't empty? If this is the case, how should I use malloc properly?

The only way I've found of remedying this has been to use the code I've commented out. It fills the newly allocated char array with \0.

Thanks!

sample output:

input words(0): we we we   
0: we
1: we
2: we

input words(1): we we we 
0: we�
1: we�
2: we

is calloc not a useful function here ? http://stackoverflow.com/questions/1538420/difference-between-malloc-and-calloc — Tony Suffolk 66, Oct 18 '14 at 21:53
Yes, `malloc` will try to reuse as much memory as possible, for cache-friendliness. — o11c, Oct 18 '14 at 22:27
Seems calloc() could have been useful if I wasn't filling out the entire array right away- in my case, zeroing out the last byte seems to have solved my problem! — Hal, Oct 19 '14 at 07:52
Something does not seem right here. free(pointer) should have instantaneously returned the memory to the OS.... — FlyingGuy, Feb 10 '15 at 05:31
Using malloc is perfectly fine, IF you know what you are doing. C is not garbage collected and it will do exactly as you ask. If you malloc and do not free before you lose the pointer you have a memory leak. Similarly if you malloc and free before you are done using that memory, you will have a segfault at best or a kernel panic at worst. — FlyingGuy, Feb 10 '15 at 05:36

score 6 · Accepted Answer · answered Oct 18 '14 at 22:10

You have two problems:

1) You are using sizeof read in the condition here:

for(i = 0; i < sizeof read; i++){

What if the input line wasn't as long as sizeof read ? You should use strlen(read) here.

2) While you allocate enough memory for the string(s), you are not 0-terminating them. You could calloc() to zero-out the entire memory allocated. Since, you are immediately writing into them, I'd prefer to use malloc() rather then needlessly zeroing out with calloc() and 0-terminate the string right after the loop with:

    for(j = 0; j < t; j++){
      string[pos][j] = read[i-(t-j)];
    }
    string[pos][j] = 0; // 0 terminates the string.

P.S.: sizeof(char) is always 1.

Great, terminating the string seems to have solved it! Nice tip on the strlen() too, I'll make use of that. Thank you! — Hal, Oct 19 '14 at 07:45

score 1 · Answer 2 · answered Oct 18 '14 at 21:53

1

You should never assume the memory given to you by malloc is zeroed.

All (c-style) strings should end with a trailing char of value 0, if you don't do this what you get is undefined behaviour.

In the realm of undefined behaviour the result is undefined! As in a compiler is free to do what it likes.

AVOID undefined behaviour.

answered Oct 18 '14 at 21:53

Alec Teal

5,770
3
23
50

Thank you, I wasn't aware malloc() had this kind of behaviour, that is very useful to know. – Hal Oct 19 '14 at 07:47

malloc() reusing memory location after free(), causing fragmentation(?)

2 Answers2