Don’t sv_force_normal in mg.c:S_save_magic

This was added to make SvREADONLY_off safe.  (I think read-only is
turned off during magic so the magic scalar itself can be set without
the sv_set* functions getting upset.)  Since SvREADONLY doesn’t mean
read-only for COWs, we don’t actually need to do sv_force_normal, but
can simply skip SvREADONLY_off for COWs.

By leaving it to sv_set* functions to do sv_force_normal, we avoid
having to copy the string buffer if it is just going to be thrown away
anyway.  S_save_magic can’t know whether the scalar will actually be
overwritten, so it has to copy the buffer.

diff --git a/mg.c b/mg.c
index 2f8c81c..89629a2 100644 (file)
--- a/mg.c
+++ b/mg.c
@@ -111,22 +111,19 @@ S_save_magic(pTHX_ I32 mgs_ix, SV *sv)
        bumped = TRUE;
     }
 
-    /* Turning READONLY off for a copy-on-write scalar (including shared
-       hash keys) is a bad idea.  */
-    if (SvIsCOW(sv))
-      sv_force_normal_flags(sv, 0);
-
     SAVEDESTRUCTOR_X(S_restore_magic, INT2PTR(void*, (IV)mgs_ix));
 
     mgs = SSPTR(mgs_ix, MGS*);
     mgs->mgs_sv = sv;
     mgs->mgs_magical = SvMAGICAL(sv);
-    mgs->mgs_readonly = SvREADONLY(sv) != 0;
+    mgs->mgs_readonly = SvREADONLY(sv) && !SvIsCOW(sv);
     mgs->mgs_ss_ix = PL_savestack_ix;   /* points after the saved destructor */
     mgs->mgs_bumped = bumped;
 
     SvMAGICAL_off(sv);
-    SvREADONLY_off(sv);
+    /* Turning READONLY off for a copy-on-write scalar (including shared
+       hash keys) is a bad idea.  */
+    if (!SvIsCOW(sv)) SvREADONLY_off(sv);
 }
 
 /*