Search code examples
cmemory-leaksrealloc

Realloc doesn't work in a while loop


I need help with my C assignment. The task is to write a program, which takes string input of unknown length. Also I need to separate words, that's why I use char**. Program stops taking input, when a special word appears. You can see my code bellow:

char **words=NULL;
 maxWords=1;
numberOfWords=0;
words=calloc(maxWords,sizeof(char **));
input=malloc(max*sizeof(char  *));

words[numberOfWords]=calloc(max,sizeof(char*));


while(stopValue){
    while((c=getchar())!='\n' && c!=' ' && c!=EOF){
        input[i++]=c;

        if(i==currentSize){
            currentSize=i+max;
            input=realloc(input,currentSize);
            words[numberOfWords]=realloc(words[numberOfWords],currentSize);
        }
    }
    input[i]='\0';
    if(strcmp(input,terminator)==0){
        break;
    }
    strcpy( words[numberOfWords],input);

    numberOfWords++;
    maxWords++;

    words=realloc(words,maxWords);
    words[numberOfWords]=calloc(max,sizeof(char*));
    currentSize=max;
    i=0;
    input=realloc(input,max);
 }

It works good, when I have only 2-3 words of input. But it fails when there are more. I think that the problem is with words=realloc(words,maxWords); this line, but I don't know what exactly.

Any help, please?


Solution

  • The second argument to calloc() should be the size of what the pointer points to, i.e. the pointed-to-type, not the size of the pointer-type itself.

    For example, suppose you want to allocate space for 10 int, assigning the result to an int *p, any of the following would be proper syntax and exhibit defined behavior:

    int *a = malloc(10 * sizeof(int));
    int *b = calloc(10, sizeof(int));
    int *c = realloc(NULL, 10*sizeof(int))
    int *d = malloc(10 * sizeof *d);
    int *e = calloc(10, sizeof *e);
    int *f = realloc(NULL, 10 * sizeof *f);
    

    A pointer-to-pointer acts no differently. If you want to allocate a sequence of pointer-to-char, identical syntax applies:

    char **a = malloc(10 * sizeof(char*));
    char **b = calloc(10, sizeof(char*));
    char **c = realloc(NULL, 10*sizeof(char*))
    char **d = malloc(10 * sizeof *d);
    char **e = calloc(10, sizeof *e);
    char **f = realloc(NULL, 10 * sizeof *f);
    

    Notice that not just the syntax, but the actual code of the last three in both of the above lists is identical, save for the pointer-type itself (the first is pointer-to-int, the second is pointer-to-pointer-to-char). That syntax takes advantage of how the sizeof operator (it isn't a function or macro; it's an operator) can be used against a variable rather than a type.

    That said, words in your code is a pointer-to-pointer-to-char. I should be allocated using similar syntax for proper sizing. Either of the following will work correctly:

    char **words = calloc(maxwords, sizeof(char*)); // right, uses specific type
    char **words = calloc(maxwords, sizeof *words); // right, gets size from var type
    

    Both do the same thing: allocate a buffer properly aligned and sized to accommodate maxwords number of char*, exactly what you would want for storing pointers to strings.

    This problem is replicated again when you do this:

    words[numberOfWords] = calloc(max, sizeof(char*)); // wrong
    

    Again, words is char**, so words[anything] is char* and as such, should be assigned an allocation based on the size of the pointed-to-type: char. Either of the following will do so:

    words[numberOfWords] = calloc(max, sizeof(char));   // right, or...
    words[numberOfWords] = calloc(max, sizeof **words); // right
    

    Ok all of that said, your suspicion that this is wrong:

    words = realloc(words, maxWords);
    

    is well-founded. The realloc function takes a byte count as the second parameter. You're passing a count of pointers, but not including the size of each pointer in the number of bytes requested. Using the syntax described earlier, this could be done as:

    words = realloc(words, maxWords * sizeof *words);
    

    or

    words = realloc(words, maxWords * sizeof(char*));
    

    Either will work, and now includes the size of each pointer, thereby calculating the correct number of bytes to request.

    Best of luck.