valgrind memory leak while reading a structure from file

Question

I keep getting this valgrind message, but I don't know what to do.

[valgrind message]

==27091== HEAP SUMMARY:
==27091==     in use at exit: 96 bytes in 4 blocks
==27091==   total heap usage: 22 allocs, 18 frees, 11,347 bytes allocated
==27091==
==27091== 24 bytes in 1 blocks are definitely lost in loss record 1 of 2
==27091==    at 0x4847581: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==27091==    by 0x1092C6: detailNew (in /home/students/s/smelov.vp/test5/lab5)
==27091==    by 0x10944F: main (in /home/students/s/smelov.vp/test5/lab5)
==27091==
==27091== 72 bytes in 3 blocks are definitely lost in loss record 2 of 2
==27091==    at 0x4847581: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==27091==    by 0x1092C6: detailNew (in /home/students/s/smelov.vp/test5/lab5)
==27091==    by 0x109534: main (in /home/students/s/smelov.vp/test5/lab5)
==27091==
==27091== LEAK SUMMARY:
==27091==    definitely lost: 96 bytes in 4 blocks
==27091==    indirectly lost: 0 bytes in 0 blocks
==27091==      possibly lost: 0 bytes in 0 blocks
==27091==    still reachable: 0 bytes in 0 blocks
==27091==         suppressed: 0 bytes in 0 blocks

[functions code]

detail detailNew(char* idBuf, char* nameBuf, int pcs){
        char *name = calloc(strlen(nameBuf) + 1, sizeof(char));
        char *id = calloc(strlen(idBuf) + 1, sizeof(char));
        strcpy(id, idBuf);
        strcpy(name, nameBuf);
        detail *item = calloc(1, sizeof(detail));
        item->id = id;
        item->name = name;
        item->pcs = pcs;
        free(id);
        free(name);
        return item[0];
}

int main(){
    char idBuf[9];
    char nameBuf[81];
    int pcs;
    FILE *details = fopen("fscanf.txt", "r");
    int inputCheck = 0;
    size_t amount = 0;
    detail *list = calloc(1, sizeof(detail));
    inputCheck = fscanf(details, "%s %s %d", idBuf, nameBuf, &pcs);
    if(inputCheck != 0 && inputCheck != -1){
        list[amount] = detailNew(idBuf, nameBuf, pcs); 
    }
    while(!feof(details)){      
        amount++;
        list = realloc(list, (amount+1) * sizeof(detail));
        inputCheck = fscanf(details, "%s %s %d", idBuf, nameBuf, &pcs);
        if(inputCheck != 0 && inputCheck != -1){
                list[amount] = detailNew(idBuf, nameBuf, pcs); 
        }
    }
    fclose(details);
    printf("lines read: %zu\n", amount);
    list = realloc(list, amount * sizeof(detail));
    ...
    free(list);
    return 0;
}

"detail" is a structure

typedef struct {
    char *id;
    char *name;
    int pcs;
}detail;

here is the "fscanf.txt" file

dafhud1t oogabooga 190
ds7fta8s exampletext 69
s8fdgt3q idkwhattowrite 78
dashf79w aaaaaaaahhh 90

I was trying somehow to change my code to be able to solve this problem. I think that a problem is that I'm not freeing "item", but I'm not able to do this, because I'm returning it to main function.

`return item[0];` This returns the content of `*item`. That pointer itself is not longer accessible after your return from this function and you cannot free it afterwards. If you only want to allocate memory for 1 element and only return the content, why do you use dynamic memory at all? Just use a local variable instead. — Gerhardh, Jan 23 '23 at 13:12
If ```fopen``` returned ```NULL```, you would be trying to read/write from ```NULL```. — Harith, Jan 23 '23 at 13:13
Here is why you should never use ["while(!feof(file))"](https://stackoverflow.com/questions/5431941/why-is-while-feoffile-always-wrong) — ryyker, Jan 23 '23 at 13:13
And ```realloc```. Neither do you check the return value of ```fscanf()```. — Harith, Jan 23 '23 at 13:14
Valgrind would give you more detail if you ran it with a build of your program that included debug information. — John Bollinger, Jan 23 '23 at 13:15
Is there really a compelling reason that you need to use pointer members in your struct? This complicates your code. Use char arrays with reasonable size to accommodate id and name. Life will be more pleasant. — ryyker, Jan 23 '23 at 13:24

John Bollinger · Accepted Answer · 2023-01-23T14:23:13.350

Here ...

        detail *item = calloc(1, sizeof(detail));

... you allocate space for a detail.

Here ...

        return item[0];

... you return the contents of that space, by value, without deallocating. This is the space that is leaked.

Since you are returning the structure by value, an easy way to fix that would be to allocate it automatically instead of dynamically. You woud then need to use the . operator instead of -> to access the members, or even assign them in an initializer. Additionally, if you do not have or cannot use strdup(), then you can roll your own.

Furthermore, your other answer (now deleted) was right that it is incorrect to free the id and name pointers inside detailNew(), because you have assigned their (pointer) values to members of the new detail. When you free the space to which id and name point, you are also freeing the space to which item->name and item-id point. You must avoid freeing that space until the program is done using it.

For example:

char *strdup(const char *src) {  // only if you don't have it already
    char *dupe = malloc(strlen(src) + 1);
    if (dupe) {
        strcpy(dupe, src);
    }
    return dupe;
}

detail detailNew(char* idBuf, char* nameBuf, int pcs){
        detail item = {
            .id = strdup(idBuf);
            .name = strdup(nameBuf);
            .pcs = pcs;
        };

        return item;
}

So I see, @Gerhardh. I've updated this answer to acknowledge that, but the key details were already summarized here, so I don't see a need for more than that. — John Bollinger, Jan 23 '23 at 14:24

valgrind memory leak while reading a structure from file

1 Answers1