This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
cleanup logic in S_sv_uncow
authorDaniel Dragan <bulk88@hotmail.com>
Fri, 5 Dec 2014 22:34:32 +0000 (17:34 -0500)
committerFather Chrysostomos <sprout@cpan.org>
Sat, 6 Dec 2014 04:14:14 +0000 (20:14 -0800)
I couldnt understand this code at first glance. Many empty conditional
blocks because of page protection ROing which isn't implemented on Win32,
caused erratic stepping in my debugger with CC optimizations *off*.

-read COWREFCNT only once
-save COWREFCNT to an auto for debugger visual inspection
-don't check len twice
-don't write sv_buf_to_rw() twice

No change in behavior is expected.

sv.c

diff --git a/sv.c b/sv.c
index 8637025..5a73d95 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -5188,22 +5188,27 @@ S_sv_uncow(pTHX_ SV * const sv, const U32 flags)
         }
         SvIsCOW_off(sv);
 # ifdef PERL_NEW_COPY_ON_WRITE
-       if (len && CowREFCNT(sv) == 0)
-           /* We own the buffer ourselves. */
-           sv_buf_to_rw(sv);
+       if (len) {
+           /* Must do this first, since the CowREFCNT uses SvPVX and
+           we need to write to CowREFCNT, or de-RO the whole buffer if we are
+           the only owner left of the buffer. */
+           sv_buf_to_rw(sv); /* NOOP if RO-ing not supported */
+           {
+               U8 cowrefcnt = CowREFCNT(sv);
+               if(cowrefcnt != 0) {
+                   cowrefcnt--;
+                   CowREFCNT(sv) = cowrefcnt;
+                   sv_buf_to_ro(sv);
+                   goto copy_over;
+               }
+           }
+           /* Else we are the only owner of the buffer. */
+        }
        else
 # endif
        {
-               
             /* This SV doesn't own the buffer, so need to Newx() a new one:  */
-# ifdef PERL_NEW_COPY_ON_WRITE
-           /* Must do this first, since the macro uses SvPVX. */
-           if (len) {
-               sv_buf_to_rw(sv);
-               CowREFCNT(sv)--;
-               sv_buf_to_ro(sv);
-           }
-# endif
+            copy_over:
             SvPV_set(sv, NULL);
             SvCUR_set(sv, 0);
             SvLEN_set(sv, 0);