This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
remove vestigial uses of PRIVSHIFT
authorDavid Mitchell <davem@iabyn.com>
Tue, 19 Jan 2016 15:19:48 +0000 (15:19 +0000)
committerDavid Mitchell <davem@iabyn.com>
Tue, 19 Jan 2016 15:19:48 +0000 (15:19 +0000)
This is a hangover from when a magic (e.g. tied) var, after calling
mg_get(), would only set the private (SVp_IOK,SVp_NOK,SVp_POK)
flags on the result, indicating that although it now had a valid integer
value (say), it wasn't a "real" integer. This was achieved by, after
calling mg_get(), shifting all the public flags by PRIVSHIFT to convert
them to private flags.

Since 5.18.0, that's not been the case (i.e. mg_get() on a tied var leaves
Svf_IOK etc set). But there are still a couple of vestigial uses of
PRIVSHIFT in the core, and this commit removes them.

For one of them, I added a test that shows that (in slightly contrived
circumnstances), it was possible for a SVp_IOK NV to be upgraded
to a SVf_IOK int on return from localisation, losing its fractional
component.

The other one, in S_sv_unmagicext_flags(), only seemed to get called
when setting a new value (e.g. from mg_set()), so its unlikely that such a
var would still have private flags set that could be incorrectly upgraded
to public.

scope.c
sv.c
t/op/taint.t

diff --git a/scope.c b/scope.c
index c08eda0..375240e 100644 (file)
--- a/scope.c
+++ b/scope.c
@@ -210,15 +210,12 @@ S_save_scalar_at(pTHX_ SV **sptr, const U32 flags)
     PERL_ARGS_ASSERT_SAVE_SCALAR_AT;
 
     osv = *sptr;
-    sv  = (flags & SAVEf_KEEPOLDELEM) ? osv : (*sptr = newSV(0));
-
-    if (SvTYPE(osv) >= SVt_PVMG && SvMAGIC(osv)) {
-       if (SvGMAGICAL(osv)) {
-           SvFLAGS(osv) |= (SvFLAGS(osv) &
-              (SVp_IOK|SVp_NOK|SVp_POK)) >> PRIVSHIFT;
-       }
-       if (!(flags & SAVEf_KEEPOLDELEM))
-           mg_localize(osv, sv, cBOOL(flags & SAVEf_SETMAGIC));
+    if (flags & SAVEf_KEEPOLDELEM)
+        sv = osv;
+    else {
+        sv  = (*sptr = newSV(0));
+        if (SvTYPE(osv) >= SVt_PVMG && SvMAGIC(osv))
+            mg_localize(osv, sv, cBOOL(flags & SAVEf_SETMAGIC));
     }
 
     return sv;
diff --git a/sv.c b/sv.c
index cea54d7..e3e5af4 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -5757,10 +5757,9 @@ S_sv_unmagicext_flags(pTHX_ SV *const sv, const int type, MGVTBL *vtbl, const U3
        if (SvMAGICAL(sv))      /* if we're under save_magic, wait for restore_magic; */
            mg_magical(sv);     /*    else fix the flags now */
     }
-    else {
+    else
        SvMAGICAL_off(sv);
-       SvFLAGS(sv) |= (SvFLAGS(sv) & (SVp_IOK|SVp_NOK|SVp_POK)) >> PRIVSHIFT;
-    }
+
     return 0;
 }
 
index eaee77a..101c6da 100644 (file)
@@ -17,7 +17,7 @@ BEGIN {
 use strict;
 use Config;
 
-plan tests => 807;
+plan tests => 808;
 
 $| = 1;
 
@@ -2378,6 +2378,19 @@ is eval { eval $::x.1 }, 1, 'reset does not taint undef';
     is_tainted $n3, "* SETn";
 }
 
+# check that localizing something with get magic (e.g. taint) doesn't
+# upgrade pIOK to IOK
+
+{
+    local our $x = 1.1 + $TAINT0;  # $x should be NOK
+    my $ix = int($x);          #          now NOK, pIOK
+    {
+        local $x = 0;
+    }
+    my $x1 = $x * 1;
+    isnt($x, 1); # it should be 1.1, not 1
+}
+
 
 # This may bomb out with the alarm signal so keep it last
 SKIP: {