This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
[perl #85026] deleting elements in a HASH iterator
authorTon Hospel <me-02@ton.iguana.be>
Mon, 16 May 2011 15:33:07 +0000 (08:33 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Fri, 20 May 2011 00:09:46 +0000 (17:09 -0700)
Internally a perl HASH is an array of single linked chains of entries.
Deleting an element means removing the correct chain entry by replacing
the pointer to the removed entry with a pointer to the next entry and
then freeing the deleted entry

However, if the deleted element is the current entry the deleted entry
is kept after removing it from the chain and the LAZYDEL flag is set.
Only on the next iteration is the element actually removed and the
iterator is set to the next entry.

However, if you delete the current iterator and then delete the next
element in the same chain the "next" pointer of the iterator is not
updated because the iterator is not on the chain anymore. That means
that when the next iteration looks up the iterator next pointer it
will point to the freed memory of the second element.

This patch fixes the places where the delete is done. Drawback is that
you may never forget to do the lazydel fixup in at any place where the
entry chain gets shortened.

hv.c

diff --git a/hv.c b/hv.c
index f9eda83..a445a2f 100644 (file)
--- a/hv.c
+++ b/hv.c
@@ -1063,8 +1063,12 @@ S_hv_delete_common(pTHX_ HV *hv, SV *keysv, const char *key, STRLEN klen,
            *oentry = HeNEXT(entry);
            if (SvOOK(hv) && entry == HvAUX(hv)->xhv_eiter /* HvEITER(hv) */)
                HvLAZYDEL_on(hv);
            *oentry = HeNEXT(entry);
            if (SvOOK(hv) && entry == HvAUX(hv)->xhv_eiter /* HvEITER(hv) */)
                HvLAZYDEL_on(hv);
-           else
+           else {
+               if (SvOOK(hv) && HvLAZYDEL(hv) &&
+                   entry == HeNEXT(HvAUX(hv)->xhv_eiter))
+                   HeNEXT(HvAUX(hv)->xhv_eiter) = HeNEXT(entry);
                hv_free_ent(hv, entry);
                hv_free_ent(hv, entry);
+           }
            xhv->xhv_keys--; /* HvTOTALKEYS(hv)-- */
            if (xhv->xhv_keys == 0)
                HvHASKFLAGS_off(hv);
            xhv->xhv_keys--; /* HvTOTALKEYS(hv)-- */
            if (xhv->xhv_keys == 0)
                HvHASKFLAGS_off(hv);
@@ -1623,8 +1627,12 @@ S_clear_placeholders(pTHX_ HV *hv, U32 items)
                *oentry = HeNEXT(entry);
                if (entry == HvEITER_get(hv))
                    HvLAZYDEL_on(hv);
                *oentry = HeNEXT(entry);
                if (entry == HvEITER_get(hv))
                    HvLAZYDEL_on(hv);
-               else
+               else {
+                   if (SvOOK(hv) && HvLAZYDEL(hv) &&
+                       entry == HeNEXT(HvAUX(hv)->xhv_eiter))
+                       HeNEXT(HvAUX(hv)->xhv_eiter) = HeNEXT(entry);
                    hv_free_ent(hv, entry);
                    hv_free_ent(hv, entry);
+               }
 
                if (--items == 0) {
                    /* Finished.  */
 
                if (--items == 0) {
                    /* Finished.  */