This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
add items checking to Internals::SvREFCNT
authorDaniel Dragan <bulk88@hotmail.com>
Sat, 10 Nov 2012 17:49:38 +0000 (12:49 -0500)
committerFather Chrysostomos <sprout@cpan.org>
Mon, 12 Nov 2012 16:35:32 +0000 (08:35 -0800)
Add item count checking to Internals::SvREFCNT in case prototype is
bypassed. Getting rid of the undef saves some instructions. Reading
SvREFCNT(sv) only once save an instruction.

lib/Internals.t
universal.c

index d3fce9c..8af04af 100644 (file)
@@ -7,7 +7,7 @@ BEGIN {
     }
 }
 
-use Test::More tests => 78;
+use Test::More tests => 82;
 
 my $ro_err = qr/^Modification of a read-only value attempted/;
 
@@ -173,4 +173,18 @@ is( Internals::SvREFCNT($foo, $big_count), $big_count,
     "set reference count unsigned");
 is( Internals::SvREFCNT($foo), $big_count, "reference count unsigned");
 
-Internals::SvREFCNT($foo, 1 );
+{
+    my @arr = Internals::SvREFCNT($foo, 1 );
+    is(scalar(@arr), 1, "SvREFCNT always returns only 1 item");
+}
+
+{
+    my $usage =  'Usage: Internals::SvREFCNT(SCALAR[, REFCOUNT])';
+    eval { &Internals::SvREFCNT();};
+    like($@, qr/\Q$usage\E/);
+    $foo = \"perl";
+    eval { &Internals::SvREFCNT($foo, 0..1);};
+    like($@, qr/\Q$usage\E/);
+    eval { &Internals::SvREFCNT($foo, 0..3);};
+    like($@, qr/\Q$usage\E/);
+}
index 8cc6e63..be06aca 100644 (file)
@@ -922,29 +922,29 @@ XS(XS_Internals_SvREADONLY)       /* This is dangerous stuff. */
     }
     XSRETURN_UNDEF; /* Can't happen. */
 }
-
 XS(XS_Internals_SvREFCNT)      /* This is dangerous stuff. */
 {
     dVAR;
     dXSARGS;
     SV * const svz = ST(0);
     SV * sv;
+    U32 refcnt;
     PERL_UNUSED_ARG(cv);
 
     /* [perl #77776] - called as &foo() not foo() */
-    if (!SvROK(svz))
+    if ((items != 1 && items != 2) || !SvROK(svz))
         croak_xs_usage(cv, "SCALAR[, REFCOUNT]");
 
     sv = SvRV(svz);
 
-    if (items == 1)
-        XSRETURN_UV(SvREFCNT(sv) - 1); /* Minus the ref created for us. */
-    else if (items == 2) {
          /* I hope you really know what you are doing. */
-        SvREFCNT(sv) = SvUV(ST(1)) + 1; /* we free one ref on exit */
-        XSRETURN_UV(SvREFCNT(sv) - 1);
-    }
-    XSRETURN_UNDEF; /* Can't happen. */
+    /* idea is for SvREFCNT(sv) to be accessed only once */
+    refcnt = items == 2 ?
+                /* we free one ref on exit */
+                (SvREFCNT(sv) = SvUV(ST(1)) + 1)
+                : SvREFCNT(sv);
+    XSRETURN_UV(refcnt - 1); /* Minus the ref created for us. */        
+
 }
 
 XS(XS_Internals_hv_clear_placehold)