Valgrind runs infinitely with realloc but code works

Question

So, I am trying to copy char per char of a file in a chunk of memory that expands accordingly with the file's size... At the end the code print all ok, but if I use valgrind it will run forever.

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>


int main(void)
{
    FILE *dict = fopen("dictionaries/large", "r");
    if (dict == NULL)
    {
        printf("Could not load the file\n");
        return 1;
    }

    // Buffer to read data.
    char *buffer = malloc(1);
    if (buffer == NULL)
    {
        return 1;
    }

    char *word = malloc(1);
    if (word == NULL)
    {
        return 1;
    }

    int i = 0;

    // Load file byte per byte. Copy char per char.
    while (fread(buffer, 1, 1, dict))
    {
        word[i] = buffer[0];
        i++;
        char *tmp = realloc(word, i + 1);
        word = tmp;
    }

    printf("%s\n", word);

    fclose(dict);
    free(buffer);
    free(word);
}

Valgrind's report:

I tried to create a temporary pointer after realloc and then assign the new pointer to the old one, but it didn't work as realloc can return a pointer to another address.

Welcome to Stack Overflow! Please post code, data, and results as text, not screenshots. [Why should I not upload images of code/data/errors?](https://meta.stackoverflow.com/questions/285551/why-should-i-not-upload-images-of-code-data-errors) http://idownvotedbecau.se/imageofcode — Barmar, Mar 01 '23 at 20:17
Why are you using a dynamic array for `buffer`? You never read more than one character, so it doesn't need to be an array. And it's not dynamically sized, so it doesn't need to use `malloc()` — Barmar, Mar 01 '23 at 20:20
Your code will run forever if `fread` returns an error. Also, you can only use the `%s` format specifier for C-style strings, not arbitrary arrays of characters. — David Schwartz, Mar 01 '23 at 20:20
The reason to use a temporary pointer for `realloc()` is so you can test the result before assigning it back to the original variable. Where is your `if(temp == NULL)` check? — Barmar, Mar 01 '23 at 20:22
Loading a file in one byte at a time is preposterous and super slow. Consider using a reasonable buffer size like 1KB or more. Remember, a 1 byte C string holds **zero** useful data. If you're intending to read in the whole file anyway, you can skip the read pointer to the end, grab the offset, allocate that much memory, rewind the pointer and read it in with one operation. This is **way faster**. — tadman, Mar 01 '23 at 20:23
@Barmar I see. Thanks for the explanation. By now I don't know how to read data without fread and as per the manual the first argument should be a pointer to a location in memory, but using *char just gave me segmentation core. I want it to be an array because the file itself was a bunch of words, then the task is to isolate them and later transfer every single word to a hash table. Sorry, I didn't specify that in the question — DumbDumbie, Mar 01 '23 at 20:34
If you want to read one character use `fgetc()`. You can also use `char c; fread(&c, 1, 1, dict)` — Barmar, Mar 01 '23 at 20:40
Thanks to everyone whom participated! Feel like I am trying to do something that is above my level, I should go back to study. — DumbDumbie, Mar 01 '23 at 21:17

score 0 · Accepted Answer · answered Mar 01 '23 at 21:01

It is always easier to divide program into logical bits and place them in functions.

char *addToBuff(char *buff, int ch, size_t *size)
{
    //working on local variable buff. No need for a temporary variable
    buff = realloc(buff, *size + 1);
    if(buff)
    {
        buff[*size] = ch;
        *size += 1;
    }
    return buff;
}

char *readToBuff(FILE *fi, size_t *size)
{
    char *buff = NULL;
    int ch;
    *size = 0;
    while((ch = fgetc(fi)) != EOF)
    {
        //to avoid memory leak - temporary variable needed
        char *tmp = addToBuff(buff, ch, size);
        if(!tmp) {/* handle allocation error */}
        else buff = tmp;
    }
    return buff;
}


int main(void)
{
    size_t size = 0;
    char *buff = readToBuff(stdin, &size);

    printf("read %zu chars. Buff address: '%p'\n", size, (void*)buff);
    free(buff);
}

https://godbolt.org/z/n56xsd3Pd

Thanks for this! I will use this to study! Will have a little hard time but will be worth it! — DumbDumbie, Mar 01 '23 at 21:18

Valgrind runs infinitely with realloc but code works

1 Answers1