Search code examples
carraysmemorydynamicrealloc

realloc() fail when expanding dynamic char** array in C


I'm writing a script that needs to read every line in a very large file (110,000L) into a dynamic array (and then it does something that is irrelevant to my question). Here's the relevant code:

 FILE *orig;
 FILE *dest;
    orig = fopen("../../dicts/mpron/pronDict.txt","r+");
    dest = fopen("../../dicts/editedPronDict.txt","w");

    char **arr = malloc(sizeof(char *)*128);
    int size = 0;
    int capacity = 128;

    char *inputLine = malloc(sizeof(char)*128);
    fgets(inputLine,128,orig);

    while(!feof(orig)){

            arr[size] = malloc(sizeof(char)*128);
            strcpy(arr[size],inputLine);
            size++;

            if(size == capacity){
                    realloc(arr,sizeof(char *)*(capacity*2));
                    capacity*=2;
                    fprintf(stderr,"New size: %i\n",capacity);
            }
    }

It's pretty basic stuff, and I thought it would run perfectly. However, I get this output:

./organizeMpron
New size: 256
New size: 512
organizeMpron(2088,0x7fff7c53e300) malloc: *** error for object  
0x7fc2aa001200: pointer being realloc'd was not allocated
*** set a breakpoint in malloc_error_break to debug  
Abort trap: 6

So it only doubles in capacity twice before crashing. I tried changing the initial capacity of the array to 64, and strangely enough I get a completely different output:

./organizeMpron
New size: 128
New size: 256
New size: 512
New size: 1024
New size: 2048
New size: 4096
New size: 8192
New size: 16384
New size: 32768
New size: 65536
New size: 131072
Segmentation fault: 11

Does anyone have any clue what might be going on here? I've written dynamic collections in this fashion several times and I have no idea where this code is breaking. Thank you in advance.


Solution

  • realloc(arr,sizeof(char *)*(capacity*2));
    

    Might reallocate arr, but it doesn't change the value of arr, which still points at the (now deallocated) original block. You probably meant

    arr = realloc(arr,sizeof(char *)*(capacity*2));
    

    although that suffers from a memory leak in the case that the reallocation fails.

    See the realloc manpage for details


    It seems a little silly to allocate 128 bytes for every line, unless you believe that all the lines are the same length. You could just allocate the number of bytes you need for the line (plus a terminating NUL). (Or you could read all the lines into one contiguous memory area and then use something like strtok to split at all the newline characters and construct your array, which would be a lot more efficient, but that's way out of scope.)

    Alternatively, if you're going to allocate a fixed buffer size for every line regardless of how long it is, there is no point reading into a temporary buffer and then copying the temporary buffer into the array. Just read directly into the appropriate array buffer.

    Finally, while (!feof(f)) is almost always wrong.