Realloc overwriting contents of array?

Question

I'm trying to read a file into an array of strings using getline() and realloc(). I have used very similar code in the past for tokenizing strings and everything worked correctly. I'll consider the sample input file as

1
2
3
4
5

Here's the code:

char** read_input(const char* input_file, char* line){

    FILE *fp;
    size_t len = 0;
    size_t nums = 0;
    ssize_t read;
    char** res = NULL;

    if ((fp = fopen(input_file, "r")) == NULL){
        printf("Incorrect file\n", strerror(errno));
        exit(EXIT_FAILURE);
    }

    while ((read = getline(&line, &len, fp)) != -1){
        if ((res = realloc(res, sizeof(char*) * ++nums)) == NULL)
            exit(EXIT_FAILURE);

        char to_strip[sizeof(read) * sizeof(char)];
        strcpy(to_strip, line);
        if (line[read - 1] == '\n')
            to_strip[read - 1] = 0;
        else
            line[read] = 0;

        res[nums - 1] = to_strip;
    }
    free(line);

    if ((res = realloc(res, sizeof(char*) * (nums + 1))) == NULL)
        exit(EXIT_FAILURE);

    res[nums - 1] = 0;

    return res;
}

After the loop, if I print the contents of the array, I get:

5
5
5
5
5

despite the fact that if I call print inside the loop, after each assignment to res, I get the right numbers. This is really stumping me, because I can't see what could be wrong except with realloc, but I thought that realloc preserved array contents. Thanks.

Answer 1

You're busy invoking undefined behaviour because you're storing a pointer to to_strip in the reallocated array each time, and the pointer goes out of scope each iteration of the loop, and gets overwritten on each iteration which is why you see the same value at the end. If you printed all the values in the loop rather than just the current value, you'd see first 1, then 2 2, then 3 3 3, then 4 4 4 4 and finally 5 5 5 5 5. If you did enough work after returning from this function before printing the results, you'd see garbage as the space would be used for other purposes.

You need to make copies of the lines you store in your reallocated array. The simplest is to use strdup():

res[nums - 1] = strdup(to_strip);

Don't forget to release the strings as well as the array of pointers to the strings.

Don't forget to close the file you opened before you return.

It seems odd to pass line into the function. It must either be a null pointer or pointing to space that can be passed to realloc(). Since you then free() the space before returning, the calling function needs to know that you've released the space it passed you — and since you didn't know how big it was, you told getline() it was of size zero, so it had to be released. The interface would be cleaner without that parameter; use a local char *line = 0; at the start of the function.

Answer 2

This is a problem:

ssize_t read;

char to_strip[sizeof(read) * sizeof(char)];
strcpy(to_strip, line);

sizeof(read) - probably 4 or 8 - is a strange amount to allocate for a buffer that you are copying a string into. I think you meant char to_strip[ read + 1 ];. However, later on you have the line:

res[nums - 1] = to_strip;

which places a pointer to to_strip into res. However, to_strip ceases to exist at the end of the for loop, so these will be wild pointers. If your intent is to store all of the text read from the file for later access then you will need to allocate memory for each line.

Jonathan Leffler's suggestion of strdup is probably the simplest solution to this; thanks to him for clearing up my erroneous use of getline.

You could also do away with to_strip entirely, as you could just overwrite the \n directly in line.