Search code examples
ciorealloc

Realloc overwriting contents of array?


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.


Solution

  • 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.