memory leak with repeated strdup's

Question

I am creating an array of filenames obtained from a linux_dirent structure (d). At each iteration of a loop a filename is obtained using

d_entry = strdup(d->d_name);

and a pointer to this is added to the array:

srcList[aSz] = d_entry;

As the array of pointers needs to have valid memory to point to I can't do this:

d_entry = strdup(d->d_name);
srcList[aSz] = d_entry;
free(d_entry);

Using free(d_entry) after the last use of the array only frees the memory allocated by strdup/malloc for the last instance of d_entry.

Valgrind confirms the memory leak.

Is there a way of dealing with this or should I look at using say memcpy to move the filenames to a separate buffer before creating the pointers in the array.

The core loop:

   for (bpos = 0; bpos < nread;) {
       d = (struct linux_dirent *) (buf + bpos);
       d_type = *(buf + bpos + d->d_reclen - 1);
       if( d->d_ino != 0 && d_type == DT_REG || d_type == DT_UNKNOWN ) {

           /* get directory entry */
            d_entry = strdup(d->d_name); // << repeat allocations here

           /* save pointer to filename in array 'srcList' */
                srcList[aSz] = d_entry;
                aSz++;
       }
       if ( aSz == DAY_COUNT +1 ) break;
       bpos += d->d_reclen;
   }

Make sure you `free()` the entries in `srcList[]` when you no longer need them? — EOF, Feb 23 '16 at 00:11
Why don't you just iterate your array later and free everything? — nneonneo, Feb 23 '16 at 00:14
You need to call `free` once for each call to `strdup`, sooner or later — M.M, Feb 23 '16 at 00:18
@nneonneo How do I free everything when I don't have a pointer to the multiple d_name memory blocks — anita2R, Feb 23 '16 at 00:20
@M.M That's what I tried - but then the pointers in the array are pointing to memory that could/will be reallocated. — anita2R, Feb 23 '16 at 00:23
@nneonneo Thanks - of course I have the pointers already in the array. I am new to C and this did not occur to me. I will give that a go - will have to be tomorrow now. — anita2R, Feb 23 '16 at 00:27
@anita2R You're not going to free the memory until you don't need it any more. So you're never going to free it and then try to use it, because if you did, then that means you actually did need it and therefore shouldn't have freed it yet. — user253751, Feb 23 '16 at 01:16
@ immibis I am not trying to free the memory allocated to the multiple instances of d_entry until I have transferred the data out of the array. At that point I don't need the array (or the data it points to). I am trying to free the memory once srcList[] is no longer in use. — anita2R, Feb 23 '16 at 04:29

score 1 · Accepted Answer · answered Feb 23 '16 at 10:24

1

As discussed in comments, the leak is fixed by

for ( i=0; i< size;i++)
   free( srcList[i] );

When array is no longer needed

answered Feb 23 '16 at 10:24

mksteve

12,614
3
28
50

That works, but as some elements in the array were not populated I used if(srcList[n]) free(srcList[n]); . The array was initialized to nulls and so only populated elements were used for free. – anita2R Feb 23 '16 at 14:34
@anita2R I'm four years late, but free() is safe to call on NULL if implementation follows ISO https://en.cppreference.com/w/c/memory/free – Hunter Kohler Nov 17 '21 at 00:52

memory leak with repeated strdup's

1 Answers1