Search code examples
cmemory-managementmallocvalgrindfree

Properly freeing memory space malloced in other function


Edit : the problem isn't what I thought it was, check answers before reading my (long) question will spare you some time.

i've been valgrinding my little C project for school (basically a language detector), but there's a set of pointers I still have problems with according to valgrind. I think the problem is classic, it could be resumed in : How should I free a variable i malloced in another function when I (think I) have no choice ?

Here are the relevant parts of the two functions which cause me this problem. Basically, get_modeles_names gathers the filenames holding the right substring within them from my "collection" file; then det_langue is processing them. Here is get_modeles_names :

    void get_modeles_names(int mode, char ** modeles)
{
        FILE * collection = get_collection_file();
        int i = 0;
        char * line = NULL;
        size_t size = 0;

        while (getline(&line, &size, collection) != -1) {
            if (strstr(line, entete)) {
                modeles[i] = (char*) malloc(strlen(line) * sizeof (char));
                modeles[i] = line;

                modeles[i][strlen(modeles[i]) - 1] = '\0';
                //replaces the final '\n' by '\0'

                i++;
                line = NULL;
            }
        }

        if (line)
            free(line);

        fclose(collection);
}

And here is det_langue :

void det_langue(char * w)
{
    int nb_modele = nb_modeles();

    char ** modeles = (char **) malloc(nb_modele * sizeof (char *));
    get_modeles_names(mode, modeles);

    double * resultats = (double *) malloc(nb_modele * sizeof (double));

    int i;
    for (i = 0; i < nb_modele; i++) {
        resultats[i] = calculate_result (w, modeles[i]);
        }
    }

    for (i = 0; i < nb_modele; i++) {
        free(modeles[i]);
    }
    free(modeles);
    free(resultats);
}

As you can see, i malloc my char ** modeles in det_langue to the right size (i am confident the nb_modeles function works fine) , then i malloc each element of modeles in get_modeles_names ; but since i need to process them later on, i can't free them (or don't know how) where i malloced them, i free them later, in the end of det_langue. I would think it's allright, but here's the output of valgrind --leak-check=full ./projet -d pomme -m 1

==30547== Command: ./projet -d pomme -m 1
==30547== 
pomme -> french
==30547== 
==30547== HEAP SUMMARY:
==30547==     in use at exit: 113 bytes in 4 blocks
==30547==   total heap usage: 40 allocs, 36 frees, 30,906 bytes allocated
==30547== 
==30547== 113 bytes in 4 blocks are definitely lost in loss record 1 of 1
==30547==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30547==    by 0x40163E: get_modeles_names (gestion_collection_modeles.c:120)
==30547==    by 0x401919: det_langue (det_langue.c:12)
==30547==    by 0x40189E: main (Main.c:76)
==30547== 
==30547== LEAK SUMMARY:
==30547==    definitely lost: 113 bytes in 4 blocks
==30547==    indirectly lost: 0 bytes in 0 blocks
==30547==      possibly lost: 0 bytes in 0 blocks
==30547==    still reachable: 0 bytes in 0 blocks
==30547==         suppressed: 0 bytes in 0 blocks
==30547== 
==30547== For counts of detected and suppressed errors, rerun with: -v
==30547== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Line 120 is in get_modeles_names, it's : modeles[i] = (char*) malloc(strlen(line) * sizeof (char)); Also, the size of modeles is 4, which explains for the 40 allocs but only 36 frees. Also, if I go berserk and simply free modeles in the get_modeles_names function, my program obviously seg_faults, since the data i'm supposed to process don't exist anymore.

I've googled around, but couldn't find any question already asked which would answer mine .... any idea ?

Edit : the mem_leak was the one described in solution, but there is another as soon as it's solved : line leaks, which is solved by moving

if(line)
    free(line);

within the while loop.


Solution

  • I see the following problem:

            if (strstr(line, entete)) {
                modeles[i] = (char*) malloc(strlen(line) * sizeof (char));
                modeles[i] = line;
    

    Here, you first allocate memory with malloc and save the pointer to it in modeles[i] but then you overwrite the pointer with line. If you want to copy the contents of line, then use strcpy.

    Note that you do not allocate enough memory for the strcpy of line: you forget the terminating null character. It should be:

                modeles[i] = malloc(strlen(line) + 1);
    

    or:

                int len= strlen(line);
                line[len-2] = '\0';
                modeles[i] = malloc(len);
                strcpy(modeles[i], line);
    

    .. and sizeof(char) is not needed because sizeof(char) is always 1.