1
I have a function `vector_push` that pushes data to a vector like this:

    void vector_push(vector *v, void *item)
    {
        if (v->capacity == v->size)
            vector_resize(v, v->capacity * 2);
        v->data[v->size++] = item;
    }

It's a pretty standard vector_push function in c. Now, I am trying to transfer some data into it using this function:

int transfer(char temp[100], vector *v) {
        vector_push(v, strdup(&temp[0]));
        memset(temp, 0, 100);
        return 0;
    }

The problem is, strdup leaks. So, I instead wrote this:

int transfer(char temp[], vector *v) {
    char *d = malloc(strlen(&temp[0]) + 1);
    strcpy(d, &temp[0]);
    vector_push(v, d);
    memset(temp, 0, 100);

    return 0;
}

This is pretty much the strdup function.

Some explanation first. temp accrues some values that I want to push to my vector at some point. After I push, I need to reset temp and start accruing data into it once more from where I left off. However, when I memset temp, the data I pushed into my vector was lost. Obviously. So I used strdup, but then I got memory leaks.

So I decided to expand strdup and free the char* it was returning. But of course this just puts me in the same situation as last time.

I'm just not sure how to push my data to the vector then reset "temp" without leaking and without losing the data in vector. How should I do it?

Edit:

Valgrind output:

==16158==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16158==    by 0x4ED99B9: strdup (strdup.c:42)
==16158==    by 0x1099A6: transfer (tokens.c:20)
==16158==    by 0x109AF2: tokenize (tokens.c:35)
==16158==    by 0x109848: main (nush.c:246)
==16158== 
==16158== LEAK SUMMARY:
==16158==    definitely lost: 17 bytes in 3 blocks
==16158==    indirectly lost: 0 bytes in 0 blocks
==16158==      possibly lost: 0 bytes in 0 blocks
==16158==    still reachable: 0 bytes in 0 blocks
==16158==         suppressed: 0 bytes in 0 blocks
==16158== 
==16158== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
==16158== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

C - Freeing memory after strdup()

John Lexus
  • 3,576
  • 3
  • 15
  • 33
  • 1
    In what way does `strdup` leak that your function doesn't? – Daniel Pryden Feb 19 '19 at 02:48
  • Which function? Transfer? Transfer _is_ leaking now - because I removed `free(d)`. But that's my problem - if I `free(d)`, the data I push into my vector disappears. – John Lexus Feb 19 '19 at 02:49
  • 1
    Can you provide a [mcve]? Both your strcpy and strdup solutions should be fine. In both cases you will need to free that memory when you are finished with it. – Retired Ninja Feb 19 '19 at 02:50
  • @RetiredNinja they are fine in the sense that they work, but they are not fine in the sense that they leak memory. – John Lexus Feb 19 '19 at 02:51
  • 1
    @JohnLexus: There is no leak in `strdup`. The leak is in whatever code *removes* an item from your `vector` struct -- you need to be calling `free` after you are done using the pointed-to memory, and not before then. – Daniel Pryden Feb 19 '19 at 02:53
  • Look at my edits @DanielPryden – John Lexus Feb 19 '19 at 02:55
  • In either case, homegrown or strdup you need to free the memory when you are done with it, and once you free it, it is no longer yours to access. Just as you push items into your vector you must erase those items out of it and free their memory when you are done. The leak is due to incorrect management of memory, not strdup. – Retired Ninja Feb 19 '19 at 03:02
  • vector is holding a set of pointers to buffers. At some point you need to call a vector_free() function that frees those buffers. If you do that, strdup or malloc in _push won't be listed as the source of leaks. – Dave S Feb 19 '19 at 03:04
  • @DaveS I think half my code is littered with vector_free(). Just so I can make sure. I'll double check if the problem actually isnt with strdup – John Lexus Feb 19 '19 at 03:07
  • 1
    This is where a [MCVE] would help. Odds are that you're missing a vector_free but without the rest of your code we have to guess about that. – Dave S Feb 19 '19 at 03:09
  • You don't free pointers, you free objects. A memory leak is where you don't free an object. If you put a pointer to an object in the vector and then free the object, of course the pointer is useless because the object it points to was freed. – user253751 Feb 19 '19 at 03:14
  • @JohnLexus: I think you may be misinterpreting that Valgrind output. It's not saying that `strdup` or even `transfer` is the location of the leak, simply the location where the leaked object "came from". The leak is the place where you lose (or throw away) the pointer to that memory without having called `free`. Valgrind can't tell you where that is -- that's up to you to figure out. Most likely your `vector_free` implementation is buggy, or else you aren't calling it somewhere you should be. Unless you show the code to `vector_free` and the code that calls it, this question is unanswerable. – Daniel Pryden Feb 19 '19 at 20:05

1 Answers1

2

You should be freeing the objects in your vector when you are freeing your vector. For objects (like strings) with no sub-objects you can simply call free. If you want to get more fancy you can store a function pointer to a specific free function for each object and call that as you free your vector.

Jobonso
  • 21
  • 4