Memory leak in the following C code

Question

For the following sample of code there is memory leak. How can we prevent memory leak in the following case:

            1 #include <stdio.h>
            2 #include <stdlib.h>
            3 
            4 typedef struct sample_help {
            5 int *value;
            6 void **pointers;
            7 }*sample,sample_node;
            8 
            9 
            10 sample main(){
            11 sample  ABC=NULL; 
            12 sample  XYZ=NULL;
            13 sample  kanchi = NULL;
            14 
            15 ABC = malloc(sizeof(sample_node));
            16 XYZ = malloc(sizeof(sample_node));
            17 ABC->pointers = malloc(5*sizeof(void *));
            18 XYZ->pointers = malloc(5*sizeof(void *));
            19 ABC->value = malloc(5*sizeof(int));
            20 XYZ->value = malloc(5*sizeof(int));
            21 
            22 ABC->value[0] = 10;
            23 ABC->value[1] = 20;
            24 
            25 XYZ->pointers[0] = ABC;
            26 kanchi = XYZ->pointers[0];
            27 
            28 printf("::::%d\n",XYZ->pointers[0]);
            29 printf("kanchi1:::::%d\n",kanchi->value[0]);
            30 
            31 
            32 return XYZ;
            33 }
            34

Following is the output of valgrind.

==27448== 
==27448== HEAP SUMMARY:
==27448==     in use at exit: 152 bytes in 6 blocks 
==27448==   total heap usage: 6 allocs, 0 frees, 152 bytes allocated
==27448== 
==27448== 152 (16 direct, 136 indirect) bytes in 1 blocks are definitely lost in loss  record 6 of 6
==27448==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==27448==    by 0x40056B: main (test2.c:16)
==27448== 
==27448== LEAK SUMMARY:
==27448==    definitely lost: 16 bytes in 1 blocks
==27448==    indirectly lost: 136 bytes in 5 blocks
==27448==      possibly lost: 0 bytes in 0 blocks
==27448==    still reachable: 0 bytes in 0 blocks
==27448==         suppressed: 0 bytes in 0 blocks  
==27448==

**sample** is the type i would like to return in place of **int** — thetna, Sep 13 '11 at 10:24
@Mitch , if a free the allocated memory, i would not be able to return the value if **XYZ**. Can you please let me know how can I overcome this situation? — thetna, Sep 13 '11 at 10:25
I freed the **ABC**, once i free it , the value assigned to **XYZ** were lost. — thetna, Sep 13 '11 at 10:27
What are you *really* trying to do ? Why do you think you want to return a data structure from `main()` ? — Paul R, Sep 13 '11 at 10:28
@quasiverse, it should legal , since **void main()** is legal, then ofcouse the above case should be legal. — thetna, Sep 13 '11 at 10:28
@Paul R, actually this is the prototype of my real function. So as to compile and test i used main instead of my other real function. Please you may take it as some other module except for **main** that returns structure. — thetna, Sep 13 '11 at 10:30
@thetna To clarify on broken logic, it's like saying 0 squared is 0 and 1 squared is 1 so therefore, any number squared is itself. — flight, Sep 13 '11 at 10:34
@thetna: OK - in that case you should implement this is a proper function and then call it from main if you want to test it. — Paul R, Sep 13 '11 at 10:34
@Paul R, I have implemented similar functions in my real project. And in the real project also I am getting the error the way it is appearing in this code. — thetna, Sep 13 '11 at 10:40
@thetna: see my answer below for how to (a) implement this properly (without abusing main()) and (b) how to free the memory in the caller when you're done. — Paul R, Sep 13 '11 at 10:43
@Paul R, Actually, i am implementing the database, I think your code empties my entries. Is it possible me to use for future use? I mean , if i keep the data strucuted, then i may use those data for future searching or some thing like that. — thetna, Sep 13 '11 at 10:55
@thetna: yes, the whole point of the example is that you can use the data structure after the call to your function and then free it later *when you're done with it*. The line with the comment that says "`// ... do something with the data structure xyz ...` represents the part of your program that actually does something useful with the data structure. — Paul R, Sep 13 '11 at 11:23

score 3 · Accepted Answer · answered Sep 13 '11 at 10:42

Your code should really be more like this:

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

typedef struct sample_help {
    int *value;
    void **pointers;
} *sample, sample_node;

sample foo(void)
{
    sample  ABC=NULL;
    sample  XYZ=NULL;
    sample  kanchi = NULL;

    ABC = malloc(sizeof(sample_node));
    XYZ = malloc(sizeof(sample_node));
    ABC->pointers = malloc(5*sizeof(void *));
    XYZ->pointers = malloc(5*sizeof(void *));
    ABC->value = malloc(5*sizeof(int));
    XYZ->value = malloc(5*sizeof(int));

    ABC->value[0] = 10;
    ABC->value[1] = 20;

    XYZ->pointers[0] = ABC;
    kanchi = XYZ->pointers[0];

    printf("::::%d\n",XYZ->pointers[0]);
    printf("kanchi1:::::%d\n",kanchi->value[0]);

    return XYZ;
}

int main(void)
{
    // call your function
    sample xyz = foo();

    // ... do something with the data structure xyz ...

    // free memory allocated by your function
    free(xyz->pointers[0]->value);    // free ABC->value
    free(xyz->pointers[0]->pointers); // free ABC->pointers
    free(xyz->pointers[0]);           // free ABC
    free(xyz->value);                 // free XYZ->value
    free(xyz->pointers);              // free XYZ->pointers
    free(xyz);                        // free XYZ

    return 0;
}

Note that we free the data structure from within main() after we're done with it.

score 2 · Answer 2 · answered Sep 13 '11 at 10:26

2

Just free the used memory when it's no longer needed:

free(ABC->value);
free(XYZ->value);
free(ABC->pointers);
free(XYZ->pointers);
free(ABC);
free(XYZ);

Btw: When this is the whole program, it wouldn't actually matter. Since the OS reclaims all unfreed memory when a process is ended, memory that is in use until the termination of the programm is not required to be freed. However, it is good practice.

answered Sep 13 '11 at 10:26

king_nak

11,313
33
58

1

With `sample main()` we know there is no OS :P – pmg Sep 13 '11 at 10:32
If i free **XYZ** then how can I return the strucutre?I want to return the **XYZ** with the **ABC** assigned in the **pointers** of **XYZ**.Lets take an analogy that , a node is added in the root node. – thetna Sep 13 '11 at 10:33
The caller of main() in this case needs to free the memory after using the values. It is very odd to use main() in this way, can you describe more about the wider context of what you are doing? – Vicky Sep 13 '11 at 10:34
You never know when someone else will take your code (even you, 6 months later) and move it around without reviewing it. Consider for example renaming this main() function so it can be used in a deeper program. Because of that potential (even though small), I don't consider proper memory management to merely be "good practice", I consider it to be a solid requirement. – mah Sep 13 '11 at 10:35
Don't free XYZ, return a pointer to it and the function which receives it then is responsible for freeing it later. – jmsu Sep 13 '11 at 10:36
@Vickey, I have added it in the questions comment. Please refer there. I am implementing a data-base where children nodes gets keep on adding in the parent node. For it, i need to create a child node allocating memory. I am doing similar in my real code. And I am getting the memory error, the way i am getting here. – thetna Sep 13 '11 at 10:39

score 2 · Answer 3 · answered Sep 13 '11 at 10:39

Having now read your update in the comments, your module (not called main()) that allocates and returns the memory is fine.

Whatever module uses the value returned from your module, needs to free the data after use. So if you implement your module as

sample mymodule(void)
{
    sample foo = malloc(10);
    /* set up contents of foo as required */
    return foo;
}

Then the caller of mymodule would go like this:

int main (int argc, char *argv[])
{
    sample bar = mymodule();
    /* use contents of bar as required */
    free(bar);
    return 0;
}

Memory leak in the following C code

3 Answers3