views:

80

answers:

4

I have following code

 int wordLenght = 256, arrayLength = 2, i = 0, counter = 0;
 char **stringArray = NULL; 

 stringArray = calloc(arrayLength, sizeof(*stringArray));

 for(counter; counter<wordLenght; counter++) 
    stringArray[counter] = calloc(wordLenght, sizeof(stringArray));

 while(1)
 {
   printf("Input: ");
   fgets(stringArray[i], wordLenght, stdin);

   printf("stringArray[%d]: %s\n", i, stringArray[i]);

   if(i == arrayLength)
   {
     printf("Reallocation !!!\n");
     arrayLength *= 2;

     stringArray = realloc(stringArray, arrayLength*sizeof(*stringArray));

   } 

   i++;
 }    

I get this reallocation error:

*** glibc detected *** ./stringArray: realloc(): invalid next size: 0x0000000000b49010 ***
======= Backtrace: =========
/lib/libc.so.6(+0x775b6)[0x7f4dd12565b6]
/lib/libc.so.6(+0x7dd66)[0x7f4dd125cd66]
/lib/libc.so.6(realloc+0xf0)[0x7f4dd125d080]
./stringArray[0x4007f9]
/lib/libc.so.6(__libc_start_main+0xfd)[0x7f4dd11fdc4d]
./stringArray[0x400629]

What's my problem here ???

Thanks, greets

+2  A: 

You probably didn't mean sizeof(*stringArray)

In fact I believe you may want to relook at the calloc call too, I think you are allocating the size of the pointer there (word length times).

Ofir
One allocation per row is a horrible way to make a multidimensional array. You should allocate it as one block and either do the offset arithmetic yourself (always possible), or declare an appropriate variable-length-array pointer type (C99 only).
R..
He probably did mean sizeof(*stringArray). stringArray is an array of pointers to char, and that is what is being expanded to twice its size each time it runs out of space. But you're right about the 'stringArray[counter] = calloc(wordLenght, sizeof(stringArray));' calls.
Pete Kirkham
A: 

After the first time this line executes:

stringArray = realloc(stringArray, arrayLength*sizeof(*stringArray));

then stringArray[arrayLength/2] will be a garbage value - you haven't set it to point to storage for the word.

This part should use either use sizeof(**stringArray), or 1 as **stringArray is char, and the counter should only go up to arrayLength:

 for(counter; counter<wordLenght; counter++) 
     stringArray[counter] = calloc(wordLenght, sizeof(stringArray));

Instead allocate in one block:

 char* block = malloc(wordLength * arrayLength);

 for ( counter; counter < arrayLength; ++counter ) 
     stringArray[counter] = block + ( counter * wordLength );

At the moment, it's possible that there is some space after stringArray, into which you are storing the (wordLength-arrayLength) extra pointers when you calloc them, and realloc doesn't move stringArray.

It's quite probable that 0xb49010 is one of the pointers you calloc'd, and you're overwritten the memory where malloc keeps its block size..

But since you're writing off the end of stringArray, you're into undefined behaviour anyway.

Pete Kirkham
+2  A: 
 stringArray = calloc(arrayLength, sizeof(*stringArray));

Here you probably wanted to use sizeof(char*)

for(counter; counter<wordLenght; counter++) stringArray[counter] = calloc(wordLenght, sizeof(stringArray));

Here you are looping 256 times (wordLenght) but you should only 2 times (arrayLength). Additionally you probably wanted to use sizeof(char) instead of sizeof(stringArray).

if(i == arrayLength) {...}

This check should be done before you call fgets, because right now you are firstly using memory and later allocate them.

Additionally after you reallocate stringArray you need to allocate rest of strings using something like this

for(counter = i; counter<arrayLength; counter++) stringArray[counter] = (char*)calloc(wordLenght, sizeof(char));

And finally you need to free all allocated memory before you exit application.

Zuljin
+1 as I missed the (i==arrayLength)
Pete Kirkham
Thank you very much! Now it works fine ;-)
leon22
Then mark it as the answer and thank you for the question :)
Michael Dorgan
There is no problem with `sizeof(*stringArray)` (although the second one should be `sizeof(**stringArray)`) - in fact this style should be preferred. I would tend to write them as `sizeof stringArray[0]` and `sizeof stringArray[0][0]` respectively.
caf
What's the best way to free the allocated memory ?
leon22
A: 

Ok here is the whole solution:

int wordLength = 256, arrayLength = 2, i = 0, counter = 0;
    char **stringArray = NULL;
    char buffer[wordLength];

    stringArray = calloc(arrayLength, sizeof(char*));
    for(counter; counter<arrayLength; counter++) stringArray[counter] = (char*)calloc(wordLength, sizeof(char));

    while(1)
    {
        if(i == arrayLength)
        {
            printf("Reallocation !!!\n");
            arrayLength *= 2;

            stringArray = realloc(stringArray, arrayLength*sizeof(char*));
            for(counter = i; counter<arrayLength; counter++) stringArray[counter] = (char*)calloc(wordLength, sizeof(char));
        }   

        printf("Input: ");
        fgets(buffer, wordLength, stdin);

        if(!strcmp(buffer,"q\n")) break; // also free here      
        else stringArray[i] = buffer;

        printf("stringArray[%d]: %s\n", i, stringArray[i]);

        i++;
    }

How is the best way to free the space ?!

greets & thank you

leon22