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

Answer 1

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.

Answer 2

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.