This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Remove SvIsCOW checks from mg.c:S_save_magic
authorFather Chrysostomos <sprout@cpan.org>
Wed, 7 Aug 2013 07:56:32 +0000 (00:56 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Sun, 11 Aug 2013 14:41:26 +0000 (07:41 -0700)
It no longer needs to worry about SvIsCOW.  This logic is left over
from when READONLY+FAKE was used for COWs.

Since it is possible for COWs to be read-only now, this logic is actu-
ally faulty, as it doesn’t temporarily stop read-only COWs from being
read-only, as it does for other read-only values.

This actually causes bugs with scalar-tied locked hash keys, which
croak on FETCH.

mg.c
t/op/tie.t

diff --git a/mg.c b/mg.c
index a6f2c53..d22f620 100644 (file)
--- a/mg.c
+++ b/mg.c
@@ -116,14 +116,12 @@ S_save_magic_flags(pTHX_ I32 mgs_ix, SV *sv, U32 flags)
     mgs = SSPTR(mgs_ix, MGS*);
     mgs->mgs_sv = sv;
     mgs->mgs_magical = SvMAGICAL(sv);
     mgs = SSPTR(mgs_ix, MGS*);
     mgs->mgs_sv = sv;
     mgs->mgs_magical = SvMAGICAL(sv);
-    mgs->mgs_readonly = SvREADONLY(sv) && !SvIsCOW(sv);
+    mgs->mgs_readonly = SvREADONLY(sv);
     mgs->mgs_ss_ix = PL_savestack_ix;   /* points after the saved destructor */
     mgs->mgs_bumped = bumped;
 
     SvFLAGS(sv) &= ~flags;
     mgs->mgs_ss_ix = PL_savestack_ix;   /* points after the saved destructor */
     mgs->mgs_bumped = bumped;
 
     SvFLAGS(sv) &= ~flags;
-    /* Turning READONLY off for a copy-on-write scalar (including shared
-       hash keys) is a bad idea.  */
-    if (!SvIsCOW(sv)) SvREADONLY_off(sv);
+    SvREADONLY_off(sv);
 }
 
 #define save_magic(a,b) save_magic_flags(a,b,SVs_GMG|SVs_SMG|SVs_RMG)
 }
 
 #define save_magic(a,b) save_magic_flags(a,b,SVs_GMG|SVs_SMG|SVs_RMG)
index e78fd5e..5710058 100644 (file)
@@ -1377,3 +1377,15 @@ sub TIEARRAY{
 tie @a, "", "$a$b";
 EXPECT
 ok
 tie @a, "", "$a$b";
 EXPECT
 ok
+########
+
+# Scalar-tied locked hash keys and copy-on-write
+use Tie::Scalar;
+tie $h{foo}, Tie::StdScalar;
+$h{foo} = __PACKAGE__;
+# Moral equivalent of Hash::Util::lock_whatever, but miniperl-compatible
+Internals::SvREADONLY($h{foo},1);
+print $h{foo}, "\n";
+
+EXPECT
+main