Re-instate initial dereference in total_size()
Nicholas Clark [Sun, 17 Apr 2011 09:28:37 +0000 (10:28 +0100)]
We most definitely can push an AV or an HV onto the pending array. Despite
having a comment saying that we can't do that, that was exactly what the
previous code *was* doing, only within the switch statement.

Previously, this code:

    my $a = [];
    my $b = \$a;
    weaken $b;
    total_size($a)

was returning a value which happened to be the size of the magic structure
(but not the object hanging from it - fixme) due to the interaction of two
different bugs. The number it pulled out of its backside was
thing_size(rv, st) - thing_size(rv, NULL), where thing_size(...) called
magic_size(...), and that returned 0 always if the st parameter was NULL,
because check_new(st, p) always returns FALSE (ie not new) if the first
parameter is NULL. Good eh?)

Size.xs
t/basic.t

diff --git a/Size.xs b/Size.xs
index 32e6269..6ed0c2b 100644 (file)
--- a/Size.xs
+++ b/Size.xs
@@ -862,12 +862,10 @@ CODE:
 
   pending_array = newAV();
 
-  /* We cannot push HV/AV directly, only the RV. So deref it
-     later (see below for "*** dereference later") and adjust here for
-     the miscalculation.
+  /* If they passed us a reference then dereference it.
      This is the only way we can check the sizes of arrays and hashes. */
   if (SvROK(thing)) {
-      RETVAL -= thing_size(aTHX_ thing, NULL);
+      thing = SvRV(thing);
   } 
 
   /* Put it on the pending array */
@@ -890,8 +888,6 @@ CODE:
         av_push(pending_array, SvRV(thing));
         } 
       TAG;break;
-
-    /* this is the "*** dereference later" part - see above */
 #if (PERL_VERSION < 11)
         case SVt_RV: TAG;
 #else
index 55e5278..b5b0838 100644 (file)
--- a/t/basic.t
+++ b/t/basic.t
@@ -96,7 +96,7 @@ cmp_ok (total_size(\&LARGE), '>', 8192,
 {
     my $a = [];
     my $b = \$a;
-    weaken $b;
-    cmp_ok(total_size($a), '>', total_size([]),
-          'making a weakref upgrades the target to PVMG and adds magic');
+    # making a weakref upgrades the target to PVMG and adds magic
+    is(total_size($a), total_size([]),
+       'Any intial reference is dereferenced and discarded');
 }