CODEWITHSUNDEEP
cmemorymemory-leaksvalgrind
rps
rps

Reputation: 1293

Why does this leak memory?

I have a function that, when called, takes a struct Pieces* field from a struct Torrent and initializes it according to the information contained in a char array.

Here is what the function call looks like (metadata.c:230):

get_pieces(t.pieces, &t.num_pieces, data);

In the get_pieces() function, I initialize t.pieces, like so (metadata.c:65):

pieces = calloc(*num_pieces, sizeof(struct Piece));

However, when I run valgrind, it says:

==8701== 76,776 bytes in 1 blocks are definitely lost in loss record 634 of 634
==8701==    at 0x4C28349: calloc (vg_replace_malloc.c:467)
==8701==    by 0x4025A4: get_pieces (metadata.c:65)
==8701==    by 0x402CDB: init_torrent (metadata.c:230)
==8701==    by 0x404018: start_torrent (torrent.c:35)
==8701==    by 0x40232E: main (main.c:17)

even though a pointer, t->pieces, is still available when my program terminates and can be free'd. Why does this leak memory?

The full source code is available at https://github.com/robertseaton/slug.

Upvotes: 2

Views: 302

Answers (3)

cnicutar
cnicutar

Reputation: 182609

You have

void get_pieces (struct Piece* pieces, uint64_t* num_pieces, char* data)
{
    /* ... */
    pieces = malloc(sizeof(struct Piece) * *num_pieces);
}

After the function is done, the object passed by the caller as the first argument is unchanged, so the result of malloc is lost. Therefore I don't see how you can free pieces.

EDIT

As user user786653 observes in the comments, you need to change your function:

void get_pieces(struct Piece **pieces...)
{
    /* ... */
    *pieces = malloc(sizeof(struct Piece) * num_pieces);
}

Upvotes: 8

lettucemode
lettucemode

Reputation: 445

In the call

get_pieces(t.pieces, &t.num_pieces, data)

you are passing the pointer t.pieces by value, so it is copied for use in the function. Since you assign to it inside the function but not outside, and these are now two different pointers, the outer pointer is never assigned. The pointer inside the function is destroyed via stack unwinding and the allocated memory sits there on the heap, forgotten.

You can fix this by changing the call to 

get_pieces(&t.pieces, &t.num_pieces, data)

Upvotes: 0

Reed Copsey
Reed Copsey

Reputation: 564323

This leaks memory because your get_pieces function is passing in a pointer to Pieces:

void get_pieces (struct Piece* pieces, ...

You then allocate memory to pieces inside of this method. When it returns, that allocated memory is no longer accessible by anything.

This is because your pointer is passed by value - reassigning the pointer does not change the calling function's copy. In order to affect that calling function, you'd have to pass a pointer to the pointer, so you can assign the original copy correctly:

void get_pieces (struct Piece** pieces, ...
     // ...
      *pieces = malloc(...

And, at the call site, pass in the address to the pointer.

Upvotes: 4

Related Questions