Fix memory leak in hfreeentries
authorFather Chrysostomos <sprout@cpan.org>
Tue, 30 Nov 2010 23:53:16 +0000 (15:53 -0800)
committerFather Chrysostomos <sprout@cpan.org>
Wed, 1 Dec 2010 00:27:13 +0000 (16:27 -0800)
commit4aa3a86749acdb989cb33981ae3dd1fde939b66a
treea12ad49dd1c24ec77b644126bf032f7f04c8a4e4
parent833f1b9305adb63ba9ffb5b9b832e8008443dcf4
Fix memory leak in hfreeentries

The change that made hfreeentries keep the name in place when iterat-
ing (2d0d1ecc) caused this statement at the end of the loop to be a
no-op for named hashes, because the HvARRAY is always present at the
end of the loop (it contains the name):

if (!HvARRAY(hv)) {
    /* Good. No-one added anything this time round.  */
    break;
}

So this line was added (by the same change) before the freeing of the
linked lists:

/* If there are no keys, there is nothing left to free. */
if (!((XPVHV*) SvANY(hv))->xhv_keys) break;

But that means that this, immediately after the freeing of the linked
lists and just before the if(!HvARRAY(hv)):

if (array != orig_array) {
    Safefree(array);
}

was not being reached, resulting in a memory leak (that Nicholas
Clark found).

This is what would happen:

On entering hfreeentries, orig_array would be assigned the value
in HvARRAY.

   HvARRAY    = original array
   orig_array = original array

Then the main loop would be entered, which would assign
HvARRAY to array:

   HvARRAY    = original array
   orig_array = original array
   array      = original array

HvARRAY would be nulled out and assigned a new value by hv_auxinit:

   HvARRAY    = first new array
   orig_array = original array
   array      = original array

Then the loop would repeat:

   HvARRAY    = first new array
   orig_array = original array
   array      = first new array

Then the HvARRAY would once more be nulled and replaced via
hv_auxinit:

   HvARRAY    = second new array
   orig_array = original array
   array      = first new array

Then the if(no keys)break; statement would be reached, exit-
ing the loop:

   HvARRAY    = second new array
   orig_array = original array
   <nothing>  = first new array

So the first new array is never freed.

This commit skips the allocation of an extra array at the beginning of
the loop if there are no keys. Then it exits early at the same spot.
hv.c