Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] Change newSVpvn("…", …) tonewSVpvs("…") #13911

Closed
p5pRT opened this issue Jun 9, 2014 · 11 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Jun 9, 2014

Migrated from rt.perl.org#122070 (status was 'resolved')

Searchable as RT122070$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @ilmari

The dual-life dists affected use Devel​::PPPort, so can safely use
newSVpvs() even though it wasn't added until Perl v5.8.9.

The newSVpvn("\\", 2) in Dumper.xs looks like a bug, since that would
create '\&*foo', which is not valid syntax. However, I could not find a
way to trigger that code path.

--
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article. - Calle Dybedahl

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @ilmari

0001-Change-newSVpvn-to-newSVpvs.patch
From 54e281f007c96ffdda474f673c8759b4595aae11 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Fri, 4 Oct 2013 16:48:40 +0100
Subject: [PATCH] =?UTF-8?q?Change=20newSVpvn("=E2=80=A6",=20=E2=80=A6)=20t?=
 =?UTF-8?q?o=20newSVpvs("=E2=80=A6")?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The dual-life dists affected use Devel::PPPort, so can safely use
newSVpvs() even though it wasn't added until Perl v5.8.9.
---
 dist/Data-Dumper/Dumper.xs      | 26 +++++++++++++-------------
 dist/Storable/Storable.xs       |  4 ++--
 ext/DynaLoader/dl_aix.xs        |  2 +-
 ext/DynaLoader/dl_dllload.xs    |  2 +-
 ext/DynaLoader/dl_dlopen.xs     |  2 +-
 ext/DynaLoader/dl_dyld.xs       |  2 +-
 ext/DynaLoader/dl_freemint.xs   |  2 +-
 ext/DynaLoader/dl_hpux.xs       |  2 +-
 ext/DynaLoader/dl_next.xs       |  2 +-
 ext/DynaLoader/dl_symbian.xs    |  2 +-
 ext/DynaLoader/dl_vms.xs        |  2 +-
 ext/DynaLoader/dl_win32.xs      |  4 ++--
 ext/DynaLoader/dlutils.c        |  2 +-
 ext/POSIX/POSIX.xs              |  2 +-
 ext/PerlIO-encoding/encoding.xs |  2 +-
 ext/PerlIO-scalar/scalar.xs     |  2 +-
 ext/Win32CORE/Win32CORE.c       |  2 +-
 ext/XS-APItest/APItest.xs       |  2 +-
 os2/OS2/OS2-Process/Process.xs  |  8 ++++----
 os2/os2.c                       |  4 ++--
 regcomp.c                       |  4 ++--
 regexec.c                       |  2 +-
 win32/win32.c                   |  6 +++---
 win32/wince.c                   |  4 ++--
 24 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/dist/Data-Dumper/Dumper.xs b/dist/Data-Dumper/Dumper.xs
index 2aeb11a..049121f 100644
--- a/dist/Data-Dumper/Dumper.xs
+++ b/dist/Data-Dumper/Dumper.xs
@@ -379,7 +379,7 @@ static SV *
 sv_x(pTHX_ SV *sv, const char *str, STRLEN len, I32 n)
 {
     if (!sv)
-	sv = newSVpvn("", 0);
+	sv = newSVpvs("");
 #ifdef DEBUGGING
     else
 	assert(SvTYPE(sv) >= SVt_PV);
@@ -539,11 +539,11 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    else {   /* store our name and continue */
 		SV *namesv;
 		if (name[0] == '@' || name[0] == '%') {
-		    namesv = newSVpvn("\\", 1);
+		    namesv = newSVpvs("\\");
 		    sv_catpvn(namesv, name, namelen);
 		}
 		else if (realtype == SVt_PVCV && name[0] == '*') {
-		    namesv = newSVpvn("\\", 2);
+		    namesv = newSVpvs("\\");
 		    sv_catpvn(namesv, name, namelen);
 		    (SvPVX(namesv))[1] = '&';
 		}
@@ -668,7 +668,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		realtype <= SVt_PVMG
 #endif
 	) {			     /* scalar ref */
-	    SV * const namesv = newSVpvn("${", 2);
+	    SV * const namesv = newSVpvs("${");
 	    sv_catpvn(namesv, name, namelen);
 	    sv_catpvn(namesv, "}", 1);
 	    if (realpack) {				     /* blessed */
@@ -689,7 +689,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    SvREFCNT_dec(namesv);
 	}
 	else if (realtype == SVt_PVGV) {		     /* glob ref */
-	    SV * const namesv = newSVpvn("*{", 2);
+	    SV * const namesv = newSVpvs("*{");
 	    sv_catpvn(namesv, name, namelen);
 	    sv_catpvn(namesv, "}", 1);
 	    sv_catpvn(retval, "\\", 1);
@@ -828,7 +828,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    if (sortkeys) {
 		if (sortkeys == &PL_sv_yes) {
 #if PERL_VERSION < 8
-                    sortkeys = sv_2mortal(newSVpvn("Data::Dumper::_sortkeys", 23));
+                    sortkeys = sv_2mortal(newSVpvs("Data::Dumper::_sortkeys"));
 #else
 		    keys = newAV();
 		    (void)hv_iterinit((HV*)ival);
@@ -1079,7 +1079,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
              * Note that we'd have to check for weak-refs, too, but this is
              * already the branch for non-refs only. */
 	    else if (val != &PL_sv_undef && (!use_sparse_seen_hash || SvREFCNT(val) > 1)) {
-		SV * const namesv = newSVpvn("\\", 1);
+		SV * const namesv = newSVpvs("\\");
 		sv_catpvn(namesv, name, namelen);
 		seenentry = newAV();
 		av_push(seenentry, namesv);
@@ -1160,8 +1160,8 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		static const char* const entries[] = { "{SCALAR}", "{ARRAY}", "{HASH}" };
 		static const STRLEN sizes[] = { 8, 7, 6 };
 		SV *e;
-		SV * const nname = newSVpvn("", 0);
-		SV * const newapad = newSVpvn("", 0);
+		SV * const nname = newSVpvs("");
+		SV * const newapad = newSVpvs("");
 		GV * const gv = (GV*)val;
 		I32 j;
 		
@@ -1260,7 +1260,7 @@ MODULE = Data::Dumper		PACKAGE = Data::Dumper         PREFIX = Data_Dumper_
 #
 # This is the exact equivalent of Dump.  Well, almost. The things that are
 # different as of now (due to Laziness):
-#   * doesn't deparse yet.
+#   * doesn't deparse yet.'
 #
 
 void
@@ -1319,7 +1319,7 @@ Data_Dumper_Dumpxs(href, ...)
 	    terse = purity = deepcopy = useqq = 0;
 	    quotekeys = 1;
 	
-	    retval = newSVpvn("", 0);
+	    retval = newSVpvs("");
 	    if (SvROK(href)
 		&& (hv = (HV*)SvRV((SV*)href))
 		&& SvTYPE(hv) == SVt_PVHV)		{
@@ -1383,7 +1383,7 @@ Data_Dumper_Dumpxs(href, ...)
 		    imax = av_len(todumpav);
 		else
 		    imax = -1;
-		valstr = newSVpvn("",0);
+		valstr = newSVpvs("");
 		for (i = 0; i <= imax; ++i) {
 		    SV *newapad;
 		
@@ -1482,7 +1482,7 @@ Data_Dumper_Dumpxs(href, ...)
 		    if (gimme == G_ARRAY) {
 			XPUSHs(sv_2mortal(retval));
 			if (i < imax)	/* not the last time thro ? */
-			    retval = newSVpvn("",0);
+			    retval = newSVpvs("");
 		    }
 		}
 		SvREFCNT_dec(postav);
diff --git a/dist/Storable/Storable.xs b/dist/Storable/Storable.xs
index 9c13e33..b59c580 100644
--- a/dist/Storable/Storable.xs
+++ b/dist/Storable/Storable.xs
@@ -2697,7 +2697,7 @@ static int store_code(pTHX_ stcxt_t *cxt, CV *cv)
 	 * blessed code references.
 	 */
 	/* Ownership of both SVs is passed to load_module, which frees them. */
-	load_module(PERL_LOADMOD_NOIMPORT, newSVpvn("B::Deparse",10), newSVnv(0.61));
+	load_module(PERL_LOADMOD_NOIMPORT, newSVpvs("B::Deparse"), newSVnv(0.61));
         SPAGAIN;
 
 	ENTER;
@@ -5630,7 +5630,7 @@ static SV *retrieve_code(pTHX_ stcxt_t *cxt, const char *cname)
 	 * prepend "sub " to the source
 	 */
 
-	sub = newSVpvn("sub ", 4);
+	sub = newSVpvs("sub ");
 	if (SvUTF8(text))
 		SvUTF8_on(sub);
 	sv_catpv(sub, SvPV_nolen(text)); /* XXX no sv_catsv! */
diff --git a/ext/DynaLoader/dl_aix.xs b/ext/DynaLoader/dl_aix.xs
index 9c98972..548e4ed 100644
--- a/ext/DynaLoader/dl_aix.xs
+++ b/ext/DynaLoader/dl_aix.xs
@@ -780,7 +780,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our 
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
 
 #endif
 
diff --git a/ext/DynaLoader/dl_dllload.xs b/ext/DynaLoader/dl_dllload.xs
index ea0a8f6..ff0c7a9 100644
--- a/ext/DynaLoader/dl_dllload.xs
+++ b/ext/DynaLoader/dl_dllload.xs
@@ -205,7 +205,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our 
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
 
 #endif
 
diff --git a/ext/DynaLoader/dl_dlopen.xs b/ext/DynaLoader/dl_dlopen.xs
index cb513ab..3a009ae 100644
--- a/ext/DynaLoader/dl_dlopen.xs
+++ b/ext/DynaLoader/dl_dlopen.xs
@@ -279,7 +279,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our 
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
 
 #endif
 
diff --git a/ext/DynaLoader/dl_dyld.xs b/ext/DynaLoader/dl_dyld.xs
index caa9467..2ed10bb 100644
--- a/ext/DynaLoader/dl_dyld.xs
+++ b/ext/DynaLoader/dl_dyld.xs
@@ -234,7 +234,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our 
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
 
 #endif
 
diff --git a/ext/DynaLoader/dl_freemint.xs b/ext/DynaLoader/dl_freemint.xs
index 6970a76..f154dcb 100644
--- a/ext/DynaLoader/dl_freemint.xs
+++ b/ext/DynaLoader/dl_freemint.xs
@@ -211,7 +211,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
     dl_resolve_using   = get_av("DynaLoader::dl_resolve_using", GV_ADDMULTI);
     dl_require_symbols = get_av("DynaLoader::dl_require_symbols", GV_ADDMULTI);
 
diff --git a/ext/DynaLoader/dl_hpux.xs b/ext/DynaLoader/dl_hpux.xs
index 4acc8c1..e089190 100644
--- a/ext/DynaLoader/dl_hpux.xs
+++ b/ext/DynaLoader/dl_hpux.xs
@@ -191,7 +191,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our 
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
     dl_resolve_using = get_av("DynaLoader::dl_resolve_using", GV_ADDMULTI);
 
 #endif
diff --git a/ext/DynaLoader/dl_next.xs b/ext/DynaLoader/dl_next.xs
index f1fb1c4..a9ddf82 100644
--- a/ext/DynaLoader/dl_next.xs
+++ b/ext/DynaLoader/dl_next.xs
@@ -337,7 +337,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our 
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
     dl_resolve_using = get_av("DynaLoader::dl_resolve_using", GV_ADDMULTI);
 
 #endif
diff --git a/ext/DynaLoader/dl_symbian.xs b/ext/DynaLoader/dl_symbian.xs
index 7f0c0d3..b509a6a 100644
--- a/ext/DynaLoader/dl_symbian.xs
+++ b/ext/DynaLoader/dl_symbian.xs
@@ -234,7 +234,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our 
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
 
 #endif
 
diff --git a/ext/DynaLoader/dl_vms.xs b/ext/DynaLoader/dl_vms.xs
index 6eb2c54..23cf11b 100644
--- a/ext/DynaLoader/dl_vms.xs
+++ b/ext/DynaLoader/dl_vms.xs
@@ -368,7 +368,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our 
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
     dl_require_symbols = get_av("DynaLoader::dl_require_symbols", GV_ADDMULTI);
 
 #endif
diff --git a/ext/DynaLoader/dl_win32.xs b/ext/DynaLoader/dl_win32.xs
index f5d56cf..ac59e11 100644
--- a/ext/DynaLoader/dl_win32.xs
+++ b/ext/DynaLoader/dl_win32.xs
@@ -48,7 +48,7 @@ OS_Error_String(pTHX)
     DWORD err = GetLastError();
     STRLEN len;
     if (!dl_error_sv)
-	dl_error_sv = newSVpvn("",0);
+	dl_error_sv = newSVpvs("");
     PerlProc_GetOSError(dl_error_sv,err);
     return SvPV(dl_error_sv,len);
 }
@@ -207,7 +207,7 @@ CLONE(...)
      * using Perl variables that belong to another thread, we create our 
      * own for this thread.
      */
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
 
 #endif
 
diff --git a/ext/DynaLoader/dlutils.c b/ext/DynaLoader/dlutils.c
index 574ccad..29d9b91 100644
--- a/ext/DynaLoader/dlutils.c
+++ b/ext/DynaLoader/dlutils.c
@@ -92,7 +92,7 @@ dl_generic_private_init(pTHX)	/* called by dl_*.xs dl_private_init() */
     char *perl_dl_nonlazy;
     MY_CXT_INIT;
 
-    MY_CXT.x_dl_last_error = newSVpvn("", 0);
+    MY_CXT.x_dl_last_error = newSVpvs("");
     dl_nonlazy = 0;
 #ifdef DL_LOADONCEONLY
     dl_loaded_files = NULL;
diff --git a/ext/POSIX/POSIX.xs b/ext/POSIX/POSIX.xs
index 9631c7d..8cb5ebf 100644
--- a/ext/POSIX/POSIX.xs
+++ b/ext/POSIX/POSIX.xs
@@ -1475,7 +1475,7 @@ tmpnam()
 	STRLEN i;
 	int len;
     CODE:
-	RETVAL = newSVpvn("", 0);
+	RETVAL = newSVpvs("");
 	SvGROW(RETVAL, L_tmpnam);
 	/* Yes, we know tmpnam() is bad.  So bad that some compilers
 	 * and linkers warn against using it.  But it is here for
diff --git a/ext/PerlIO-encoding/encoding.xs b/ext/PerlIO-encoding/encoding.xs
index fababd1..cc329d3 100644
--- a/ext/PerlIO-encoding/encoding.xs
+++ b/ext/PerlIO-encoding/encoding.xs
@@ -650,7 +650,7 @@ PROTOTYPES: ENABLE
 	Perl_warner(aTHX_ packWARN(WARN_IO), ":encoding without 'use Encode'");
 #endif
 	/* The SV is magically freed by load_module */
-	load_module(PERL_LOADMOD_NOIMPORT, newSVpvn("Encode", 6), Nullsv, Nullsv);
+	load_module(PERL_LOADMOD_NOIMPORT, newSVpvs("Encode"), Nullsv, Nullsv);
 	assert(sp == PL_stack_sp);
     }
     PUSHMARK(sp);
diff --git a/ext/PerlIO-scalar/scalar.xs b/ext/PerlIO-scalar/scalar.xs
index 8d217c9..ca5368e 100644
--- a/ext/PerlIO-scalar/scalar.xs
+++ b/ext/PerlIO-scalar/scalar.xs
@@ -46,7 +46,7 @@ PerlIOScalar_pushed(pTHX_ PerlIO * f, const char *mode, SV * arg,
 	}
     }
     else {
-	s->var = newSVpvn("", 0);
+	s->var = newSVpvs("");
     }
     SvUPGRADE(s->var, SVt_PV);
 
diff --git a/ext/Win32CORE/Win32CORE.c b/ext/Win32CORE/Win32CORE.c
index 91759e8..64104d3 100644
--- a/ext/Win32CORE/Win32CORE.c
+++ b/ext/Win32CORE/Win32CORE.c
@@ -33,7 +33,7 @@ XS(w32_CORE_all){
      * subs
      */
     const char *function  = (const char *) XSANY.any_ptr;
-    Perl_load_module(aTHX_ PERL_LOADMOD_NOIMPORT, newSVpvn("Win32",5), newSVnv(0.27));
+    Perl_load_module(aTHX_ PERL_LOADMOD_NOIMPORT, newSVpvs("Win32"), newSVnv(0.27));
     SetLastError(err);
     errno = saved_errno;
     /* mark and SP from caller are passed through unchanged */
diff --git a/ext/XS-APItest/APItest.xs b/ext/XS-APItest/APItest.xs
index a692c51..b9b18f4 100644
--- a/ext/XS-APItest/APItest.xs
+++ b/ext/XS-APItest/APItest.xs
@@ -2433,7 +2433,7 @@ my_caller(level)
         ST(4) = cop_hints_fetch_pvs(cx->blk_oldcop, "foo", 0);
         ST(5) = cop_hints_fetch_pvn(cx->blk_oldcop, "foo", 3, 0, 0);
         ST(6) = cop_hints_fetch_sv(cx->blk_oldcop, 
-                sv_2mortal(newSVpvn("foo", 3)), 0, 0);
+                sv_2mortal(newSVpvs("foo")), 0, 0);
 
         hv = cop_hints_2hv(cx->blk_oldcop, 0);
         ST(7) = hv ? sv_2mortal(newRV_noinc((SV *)hv)) : &PL_sv_undef;
diff --git a/os2/OS2/OS2-Process/Process.xs b/os2/OS2/OS2-Process/Process.xs
index 05befa0..81eb8fb 100644
--- a/os2/OS2/OS2-Process/Process.xs
+++ b/os2/OS2/OS2-Process/Process.xs
@@ -427,7 +427,7 @@ myQueryWindowText(HWND hwnd)
 	    return &PL_sv_undef;
 	return &PL_sv_no;
     }
-    sv = newSVpvn("", 0);
+    sv = newSVpvs("");
     SvGROW(sv, l + 1);
     len = QueryWindowText(hwnd, l + 1, SvPV_force(sv, n_a));
     if (len != l) {
@@ -459,7 +459,7 @@ QueryWindowSWP(HWND hwnd)
 SV *
 myQueryClassName(HWND hwnd)
 {
-    SV *sv = newSVpvn("",0);
+    SV *sv = newSVpvs("");
     STRLEN l = 46, len = 0, n_a;
 
     while (l + 1 >= len) {
@@ -534,7 +534,7 @@ myWinQueryAtomName(ATOM atom, HATOMTBL hAtomTbl)
   ULONG len = QueryAtomLength(hAtomTbl, atom);
 
   if (len) {			/* Probably always so... */
-    SV *sv = newSVpvn("",0);
+    SV *sv = newSVpvs("");
     STRLEN n_a;
 
     SvGROW(sv, len + 1);
@@ -755,7 +755,7 @@ swentries_list()
     int num, n = 0;
     STRLEN n_a;
     PSWBLOCK pswblk;
-    SV *sv = newSVpvn("",0);
+    SV *sv = newSVpvs("");
 
     if (!(_emx_env & 0x200)) 
 	     croak("swentries_list not implemented on DOS"); /* not OS/2. */
diff --git a/os2/os2.c b/os2/os2.c
index 4ae39e7..8c5e941 100644
--- a/os2/os2.c
+++ b/os2/os2.c
@@ -3855,7 +3855,7 @@ XS(XS_OS2__headerInfo)
 
 	if (size <= 0)
 	    Perl_croak(aTHX_ "OS2::_headerInfo(): unexpected size: %d", (int)size);
-	ST(0) = newSVpvn("",0);
+	ST(0) = newSVpvs("");
 	SvGROW(ST(0), size + 1);
 	sv_2mortal(ST(0));
 
@@ -3885,7 +3885,7 @@ XS(XS_OS2_libPath)
 	    Perl_croak(aTHX_ "OS2::_headerInfo(%ld,%ld,%ld,%ld) error: %s",
 		       DQHI_QUERYLIBPATHSIZE, sizeof(size), 0, 0,
 		       os2error(Perl_rc));
-	ST(0) = newSVpvn("",0);
+	ST(0) = newSVpvs("");
 	SvGROW(ST(0), size + 1);
 	sv_2mortal(ST(0));
 
diff --git a/regcomp.c b/regcomp.c
index bc5d350..e7c6662 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -5720,7 +5720,7 @@ S_concat_pat(pTHX_ RExC_state_t * const pRExC_state,
     /* if we know we have at least two args, create an empty string,
      * then concatenate args to that. For no args, return an empty string */
     if (!pat && pat_count != 1) {
-        pat = newSVpvn("", 0);
+        pat = newSVpvs("");
         SAVEFREESV(pat);
         alloced = TRUE;
     }
@@ -14088,7 +14088,7 @@ S_regclass(pTHX_ RExC_state_t *pRExC_state, I32 *flagp, U32 depth,
                         AV* this_array;
                         STRLEN cp_count = utf8_length(foldbuf,
                                                       foldbuf + foldlen);
-                        SV* multi_fold = sv_2mortal(newSVpvn("", 0));
+                        SV* multi_fold = sv_2mortal(newSVpvs(""));
 
                         Perl_sv_catpvf(aTHX_ multi_fold, "\\x{%"UVXf"}", value);
 
diff --git a/regexec.c b/regexec.c
index 6386d41..7e36001 100644
--- a/regexec.c
+++ b/regexec.c
@@ -7650,7 +7650,7 @@ Perl__get_regclass_nonbitmap_data(pTHX_ const regexp *prog,
 	
     /* If requested, return a printable version of what this swash matches */
     if (listsvp) {
-	SV* matches_string = newSVpvn("", 0);
+	SV* matches_string = newSVpvs("");
 
         /* The swash should be used, if possible, to get the data, as it
          * contains the resolved data.  But this function can be called at
diff --git a/win32/win32.c b/win32/win32.c
index 9126eec..b601452 100644
--- a/win32/win32.c
+++ b/win32/win32.c
@@ -279,7 +279,7 @@ get_regstr_from(HKEY hkey, const char *valuename, SV **svp)
 	{
 	    dTHX;
 	    if (!*svp)
-		*svp = sv_2mortal(newSVpvn("",0));
+		*svp = sv_2mortal(newSVpvs(""));
 	    SvGROW(*svp, datalen);
 	    retval = RegQueryValueEx(handle, valuename, 0, NULL,
 				     (PBYTE)SvPVX(*svp), &datalen);
@@ -358,7 +358,7 @@ get_emd_part(SV **prev_pathp, STRLEN *const len, char *trailing_path, ...)
 	/* directory exists */
 	dTHX;
 	if (!*prev_pathp)
-	    *prev_pathp = sv_2mortal(newSVpvn("",0));
+	    *prev_pathp = sv_2mortal(newSVpvs(""));
 	else if (SvPVX(*prev_pathp))
 	    sv_catpvn(*prev_pathp, ";", 1);
 	sv_catpv(*prev_pathp, mod_name);
@@ -1785,7 +1785,7 @@ win32_getenv(const char *name)
 
     needlen = GetEnvironmentVariableA(name,NULL,0);
     if (needlen != 0) {
-	curitem = sv_2mortal(newSVpvn("", 0));
+	curitem = sv_2mortal(newSVpvs(""));
         do {
             SvGROW(curitem, needlen+1);
             needlen = GetEnvironmentVariableA(name,SvPVX(curitem),
diff --git a/win32/wince.c b/win32/wince.c
index 63147cc..bbe8d31 100644
--- a/win32/wince.c
+++ b/win32/wince.c
@@ -146,7 +146,7 @@ get_regstr_from(HKEY hkey, const char *valuename, SV **svp)
 	if (retval == ERROR_SUCCESS && type == REG_SZ) {
 	    dTHX;
 	    if (!*svp)
-		*svp = sv_2mortal(newSVpvn("",0));
+		*svp = sv_2mortal(newSVpvs(""));
 	    SvGROW(*svp, datalen);
 	    retval = XCERegQueryValueExA(handle, valuename, 0, NULL,
 				     (PBYTE)SvPVX(*svp), &datalen);
@@ -226,7 +226,7 @@ get_emd_part(SV **prev_pathp, STRLEN *const len, char *trailing_path, ...)
 	/* directory exists */
 	dTHX;
 	if (!*prev_pathp)
-	    *prev_pathp = sv_2mortal(newSVpvn("",0));
+	    *prev_pathp = sv_2mortal(newSVpvs(""));
 	sv_catpvn(*prev_pathp, ";", 1);
 	sv_catpv(*prev_pathp, mod_name);
 	if(len)
-- 
2.0.0

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @ilmari

The dual-life dist affected uses Devel​::PPPort, so can safely use
sv_catpvs() even though it wasn't added until Perl v5.8.9.

--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @ilmari

0002-Change-sv_catpvn-to-sv_catpvs.patch
From 824889c74e41131cf627b432e8dcc57aac84eb06 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 9 Jun 2014 18:53:36 +0100
Subject: [PATCH 2/2] =?UTF-8?q?Change=20sv=5Fcatpvn(=E2=80=A6,=20"?=
 =?UTF-8?q?=E2=80=A6",=20=E2=80=A6)=20to=20sv=5Fcatpvs(=E2=80=A6,=20"?=
 =?UTF-8?q?=E2=80=A6")?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The dual-life dist affected uses Devel::PPPort, so can safely use
sv_catpvs() even though it wasn't added until Perl v5.8.9.
---
 dist/Data-Dumper/Dumper.xs | 92 +++++++++++++++++++++++-----------------------
 pp_ctl.c                   |  4 +-
 win32/win32.c              |  4 +-
 win32/wince.c              |  4 +-
 4 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/dist/Data-Dumper/Dumper.xs b/dist/Data-Dumper/Dumper.xs
index 049121f..44d3999 100644
--- a/dist/Data-Dumper/Dumper.xs
+++ b/dist/Data-Dumper/Dumper.xs
@@ -498,13 +498,13 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 			SV *postentry;
 			
 			if (realtype == SVt_PVHV)
-			    sv_catpvn(retval, "{}", 2);
+			    sv_catpvs(retval, "{}");
 			else if (realtype == SVt_PVAV)
-			    sv_catpvn(retval, "[]", 2);
+			    sv_catpvs(retval, "[]");
 			else
-			    sv_catpvn(retval, "do{my $o}", 9);
+			    sv_catpvs(retval, "do{my $o}");
 			postentry = newSVpvn(name, namelen);
-			sv_catpvn(postentry, " = ", 3);
+			sv_catpvs(postentry, " = ");
 			sv_catsv(postentry, othername);
 			av_push(postav, postentry);
 		    }
@@ -517,9 +517,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 			    }
 			    else {
 				sv_catpvn(retval, name, 1);
-				sv_catpvn(retval, "{", 1);
+				sv_catpvs(retval, "{");
 				sv_catsv(retval, othername);
-				sv_catpvn(retval, "}", 1);
+				sv_catpvs(retval, "}");
 			    }
 			}
 			else
@@ -584,9 +584,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	if (!purity && maxdepth > 0 && *levelp >= maxdepth) {
 	    STRLEN vallen;
 	    const char * const valstr = SvPV(val,vallen);
-	    sv_catpvn(retval, "'", 1);
-	    sv_catpvn(retval, valstr, vallen);
-	    sv_catpvn(retval, "'", 1);
+	    sv_catpvs(retval, "'");
+	    sv_catpvs(retval, valstr, vallen);
+	    sv_catpvs(retval, "'");
 	    return 1;
 	}
 
@@ -594,7 +594,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    STRLEN blesslen;
 	    const char * const blessstr = SvPV(bless, blesslen);
 	    sv_catpvn(retval, blessstr, blesslen);
-	    sv_catpvn(retval, "( ", 2);
+	    sv_catpvs(retval, "( ");
 	    if (indent >= 2) {
 		blesspad = apad;
 		apad = newSVsv(apad);
@@ -646,18 +646,18 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    rval = SvPV(sv_pattern, rlen);
 	    rend = rval+rlen;
 	    slash = rval;
-	    sv_catpvn(retval, "qr/", 3);
+	    sv_catpvs(retval, "qr/");
 	    for (;slash < rend; slash++) {
 	      if (*slash == '\\') { ++slash; continue; }
 	      if (*slash == '/') {    
 		sv_catpvn(retval, rval, slash-rval);
-		sv_catpvn(retval, "\\/", 2);
+		sv_catpvs(retval, "\\/");
 		rlen -= slash-rval+1;
 		rval = slash+1;
 	      }
 	    }
 	    sv_catpvn(retval, rval, rlen);
-	    sv_catpvn(retval, "/", 1);
+	    sv_catpvs(retval, "/");
 	    if (sv_flags)
 	      sv_catsv(retval, sv_flags);
 	} 
@@ -670,17 +670,17 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	) {			     /* scalar ref */
 	    SV * const namesv = newSVpvs("${");
 	    sv_catpvn(namesv, name, namelen);
-	    sv_catpvn(namesv, "}", 1);
+	    sv_catpvs(namesv, "}");
 	    if (realpack) {				     /* blessed */
-		sv_catpvn(retval, "do{\\(my $o = ", 13);
+		sv_catpvs(retval, "do{\\(my $o = ");
 		DD_dump(aTHX_ ival, SvPVX_const(namesv), SvCUR(namesv), retval, seenhv,
 			postav, levelp,	indent, pad, xpad, apad, sep, pair,
 			freezer, toaster, purity, deepcopy, quotekeys, bless,
 			maxdepth, sortkeys, use_sparse_seen_hash, useqq);
-		sv_catpvn(retval, ")}", 2);
+		sv_catpvs(retval, ")}");
 	    }						     /* plain */
 	    else {
-		sv_catpvn(retval, "\\", 1);
+		sv_catpvs(retval, "\\");
 		DD_dump(aTHX_ ival, SvPVX_const(namesv), SvCUR(namesv), retval, seenhv,
 			postav, levelp,	indent, pad, xpad, apad, sep, pair,
 			freezer, toaster, purity, deepcopy, quotekeys, bless,
@@ -691,8 +691,8 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	else if (realtype == SVt_PVGV) {		     /* glob ref */
 	    SV * const namesv = newSVpvs("*{");
 	    sv_catpvn(namesv, name, namelen);
-	    sv_catpvn(namesv, "}", 1);
-	    sv_catpvn(retval, "\\", 1);
+	    sv_catpvs(namesv, "}");
+	    sv_catpvs(retval, "\\");
 	    DD_dump(aTHX_ ival, SvPVX_const(namesv), SvCUR(namesv), retval, seenhv,
 		    postav, levelp,	indent, pad, xpad, apad, sep, pair,
 		    freezer, toaster, purity, deepcopy, quotekeys, bless,
@@ -710,11 +710,11 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    (void)strcpy(iname, name);
 	    inamelen = namelen;
 	    if (name[0] == '@') {
-		sv_catpvn(retval, "(", 1);
+		sv_catpvs(retval, "(");
 		iname[0] = '$';
 	    }
 	    else {
-		sv_catpvn(retval, "[", 1);
+		sv_catpvs(retval, "[");
 		/* omit "->" in $foo{bar}->[0], but not in ${$foo}->[0] */
 		/*if (namelen > 0
 		    && name[namelen-1] != ']' && name[namelen-1] != '}'
@@ -761,7 +761,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		if (indent >= 3) {
 		    sv_catsv(retval, totpad);
 		    sv_catsv(retval, ipad);
-		    sv_catpvn(retval, "#", 1);
+		    sv_catpvs(retval, "#");
 		    sv_catsv(retval, ixsv);
 		}
 		sv_catsv(retval, totpad);
@@ -771,7 +771,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 			freezer, toaster, purity, deepcopy, quotekeys, bless,
 			maxdepth, sortkeys, use_sparse_seen_hash, useqq);
 		if (ix < ixmax)
-		    sv_catpvn(retval, ",", 1);
+		    sv_catpvs(retval, ",");
 	    }
 	    if (ixmax >= 0) {
 		SV * const opad = sv_x(aTHX_ Nullsv, SvPVX_const(xpad), SvCUR(xpad), (*levelp)-1);
@@ -780,9 +780,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		SvREFCNT_dec(opad);
 	    }
 	    if (name[0] == '@')
-		sv_catpvn(retval, ")", 1);
+		sv_catpvs(retval, ")");
 	    else
-		sv_catpvn(retval, "]", 1);
+		sv_catpvs(retval, "]");
 	    SvREFCNT_dec(ixsv);
 	    SvREFCNT_dec(totpad);
 	    Safefree(iname);
@@ -798,11 +798,11 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	
 	    SV * const iname = newSVpvn(name, namelen);
 	    if (name[0] == '%') {
-		sv_catpvn(retval, "(", 1);
+		sv_catpvs(retval, "(");
 		(SvPVX(iname))[0] = '$';
 	    }
 	    else {
-		sv_catpvn(retval, "{", 1);
+		sv_catpvs(retval, "{");
 		/* omit "->" in $foo[0]->{bar}, but not in ${$foo}->{bar} */
 		if ((namelen > 0
 		     && name[namelen-1] != ']' && name[namelen-1] != '}')
@@ -810,16 +810,16 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		        && (name[1] == '{'
 			    || (name[0] == '\\' && name[2] == '{'))))
 		{
-		    sv_catpvn(iname, "->", 2);
+		    sv_catpvs(iname, "->");
 		}
 	    }
 	    if (name[0] == '*' && name[namelen-1] == '}' && namelen >= 8 &&
 		(instr(name+namelen-8, "{SCALAR}") ||
 		 instr(name+namelen-7, "{ARRAY}") ||
 		 instr(name+namelen-6, "{HASH}"))) {
-		sv_catpvn(iname, "->", 2);
+		sv_catpvs(iname, "->");
 	    }
-	    sv_catpvn(iname, "{", 1);
+	    sv_catpvs(iname, "{");
 	    totpad = newSVsv(sep);
 	    sv_catsv(totpad, pad);
 	    sv_catsv(totpad, apad);
@@ -894,7 +894,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
                }
 
 		if (i)
-		    sv_catpvn(retval, ",", 1);
+		    sv_catpvs(retval, ",");
 
 		if (sortkeys) {
 		    char *key;
@@ -961,7 +961,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		}
                 sname = newSVsv(iname);
                 sv_catpvn(sname, nkey, nlen);
-                sv_catpvn(sname, "}", 1);
+                sv_catpvs(sname, "}");
 
 		sv_catsv(retval, pair);
 		if (indent >= 2) {
@@ -994,14 +994,14 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		SvREFCNT_dec(opad);
 	    }
 	    if (name[0] == '%')
-		sv_catpvn(retval, ")", 1);
+		sv_catpvs(retval, ")");
 	    else
-		sv_catpvn(retval, "}", 1);
+		sv_catpvs(retval, "}");
 	    SvREFCNT_dec(iname);
 	    SvREFCNT_dec(totpad);
 	}
 	else if (realtype == SVt_PVCV) {
-	    sv_catpvn(retval, "sub { \"DUMMY\" }", 15);
+	    sv_catpvs(retval, "sub { \"DUMMY\" }");
 	    if (purity)
 		warn("Encountered CODE ref, using dummy placeholder");
 	}
@@ -1017,7 +1017,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		SvREFCNT_dec(apad);
 		apad = blesspad;
 	    }
-	    sv_catpvn(retval, ", '", 3);
+	    sv_catpvs(retval, ", '");
 
 	    plen = strlen(realpack);
 	    pticks = num_q(realpack, plen);
@@ -1036,11 +1036,11 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    else {
 	        sv_catpvn(retval, realpack, strlen(realpack));
 	    }
-	    sv_catpvn(retval, "' )", 3);
+	    sv_catpvs(retval, "' )");
 	    if (toaster && SvPOK(toaster) && SvCUR(toaster)) {
-		sv_catpvn(retval, "->", 2);
+		sv_catpvs(retval, "->");
 		sv_catsv(retval, toaster);
-		sv_catpvn(retval, "()", 2);
+		sv_catpvs(retval, "()");
 	    }
 	}
 	SvREFCNT_dec(ipad);
@@ -1065,9 +1065,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		if ((svp = av_fetch(seenentry, 0, FALSE)) && (othername = *svp)
 		    && (svp = av_fetch(seenentry, 2, FALSE)) && *svp && SvIV(*svp) > 0)
 		{
-		    sv_catpvn(retval, "${", 2);
+		    sv_catpvs(retval, "${");
 		    sv_catsv(retval, othername);
-		    sv_catpvn(retval, "}", 1);
+		    sv_catpvs(retval, "}");
 		    return 1;
 		}
 	    }
@@ -1178,7 +1178,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 			
 			sv_setsv(nname, postentry);
 			sv_catpvn(nname, entries[j], sizes[j]);
-			sv_catpvn(postentry, " = ", 3);
+			sv_catpvs(postentry, " = ");
 			av_push(postav, postentry);
 			e = newRV_inc(e);
 			
@@ -1200,7 +1200,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    }
 	}
 	else if (val == &PL_sv_undef || !SvOK(val)) {
-	    sv_catpvn(retval, "undef", 5);
+	    sv_catpvs(retval, "undef");
 	}
 #ifdef SvVOK
 	else if (SvMAGICAL(val) && (mg = mg_find(val, 'V'))) {
@@ -1455,7 +1455,7 @@ Data_Dumper_Dumpxs(href, ...)
 		    if (postlen >= 0 || !terse) {
 			sv_insert(valstr, 0, 0, " = ", 3);
 			sv_insert(valstr, 0, 0, SvPVX_const(name), SvCUR(name));
-			sv_catpvn(valstr, ";", 1);
+			sv_catpvs(valstr, ";");
 		    }
 		    sv_catsv(retval, pad);
 		    sv_catsv(retval, valstr);
@@ -1469,13 +1469,13 @@ Data_Dumper_Dumpxs(href, ...)
 			    if (svp && (elem = *svp)) {
 				sv_catsv(retval, elem);
 				if (i < postlen) {
-				    sv_catpvn(retval, ";", 1);
+				    sv_catpvs(retval, ";");
 				    sv_catsv(retval, sep);
 				    sv_catsv(retval, pad);
 				}
 			    }
 			}
-			sv_catpvn(retval, ";", 1);
+			sv_catpvs(retval, ";");
 			    sv_catsv(retval, sep);
 		    }
 		    sv_setpvn(valstr, "", 0);
diff --git a/pp_ctl.c b/pp_ctl.c
index 6a619ce..b559c5a 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -3649,7 +3649,7 @@ S_doopen_pm(pTHX_ SV *name)
 	Stat_t pmcstat;
 
 	SvSetSV_nosteal(pmcsv,name);
-	sv_catpvn(pmcsv, "c", 1);
+	sv_catpvs(pmcsv, "c");
 
 	if (PerlLIO_stat(SvPV_nolen_const(pmcsv), &pmcstat) >= 0)
 	    return check_type_and_open(pmcsv);
@@ -4078,7 +4078,7 @@ PP(pp_require)
 			sv_catpv(msg, " (you may need to install the ");
 			for (c = name; c < e; c++) {
 			    if (*c == '/') {
-				sv_catpvn(msg, "::", 2);
+				sv_catpvs(msg, "::");
 			    }
 			    else {
 				sv_catpvn(msg, c, 1);
diff --git a/win32/win32.c b/win32/win32.c
index b601452..26d419e 100644
--- a/win32/win32.c
+++ b/win32/win32.c
@@ -360,7 +360,7 @@ get_emd_part(SV **prev_pathp, STRLEN *const len, char *trailing_path, ...)
 	if (!*prev_pathp)
 	    *prev_pathp = sv_2mortal(newSVpvs(""));
 	else if (SvPVX(*prev_pathp))
-	    sv_catpvn(*prev_pathp, ";", 1);
+	    sv_catpvs(*prev_pathp, ";");
 	sv_catpv(*prev_pathp, mod_name);
 	if(len)
 	    *len = SvCUR(*prev_pathp);
@@ -418,7 +418,7 @@ win32_get_xlib(const char *pl, const char *xlib, const char *libname,
 	sv1 = sv2;
     } else if (sv2) {
         dTHX;
-	sv_catpvn(sv1, ";", 1);
+	sv_catpv(sv1, ";");
 	sv_catsv(sv1, sv2);
     }
 
diff --git a/win32/wince.c b/win32/wince.c
index bbe8d31..271df2b 100644
--- a/win32/wince.c
+++ b/win32/wince.c
@@ -227,7 +227,7 @@ get_emd_part(SV **prev_pathp, STRLEN *const len, char *trailing_path, ...)
 	dTHX;
 	if (!*prev_pathp)
 	    *prev_pathp = sv_2mortal(newSVpvs(""));
-	sv_catpvn(*prev_pathp, ";", 1);
+	sv_catpvs(*prev_pathp, ";");
 	sv_catpv(*prev_pathp, mod_name);
 	if(len)
 	    *len = SvCUR(*prev_pathp);
@@ -286,7 +286,7 @@ win32_get_xlib(const char *pl, const char *xlib, const char *libname,
     if (!sv1) {
 	sv1 = sv2;
     } else if (sv2) {
-	sv_catpvn(sv1, ";", 1);
+	sv_catpvs(sv1, ";");
 	sv_catsv(sv1, sv2);
     }
 
-- 
2.0.0

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @ilmari

Oops, the previous patch had a spurious s/pvn/pvs/ for a non-constant
item, so failed to compile. Here's an updated version which builds and
passes all tests.

--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 9, 2014

From @ilmari

0002-Change-sv_catpvn-to-sv_catpvs.patch
From 8f31ba5b847a7cbcac0eb30cf72ca423c4ec74f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 9 Jun 2014 18:53:36 +0100
Subject: [PATCH 2/2] =?UTF-8?q?Change=20sv=5Fcatpvn(=E2=80=A6,=20"?=
 =?UTF-8?q?=E2=80=A6",=20=E2=80=A6)=20to=20sv=5Fcatpvs(=E2=80=A6,=20"?=
 =?UTF-8?q?=E2=80=A6")?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The dual-life dist affected uses Devel::PPPort, so can safely use
sv_catpvs() even though it wasn't added until Perl v5.8.9.
---
 dist/Data-Dumper/Dumper.xs | 90 +++++++++++++++++++++++-----------------------
 pp_ctl.c                   |  4 +--
 win32/win32.c              |  4 +--
 win32/wince.c              |  4 +--
 4 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/dist/Data-Dumper/Dumper.xs b/dist/Data-Dumper/Dumper.xs
index 049121f..6356501 100644
--- a/dist/Data-Dumper/Dumper.xs
+++ b/dist/Data-Dumper/Dumper.xs
@@ -498,13 +498,13 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 			SV *postentry;
 			
 			if (realtype == SVt_PVHV)
-			    sv_catpvn(retval, "{}", 2);
+			    sv_catpvs(retval, "{}");
 			else if (realtype == SVt_PVAV)
-			    sv_catpvn(retval, "[]", 2);
+			    sv_catpvs(retval, "[]");
 			else
-			    sv_catpvn(retval, "do{my $o}", 9);
+			    sv_catpvs(retval, "do{my $o}");
 			postentry = newSVpvn(name, namelen);
-			sv_catpvn(postentry, " = ", 3);
+			sv_catpvs(postentry, " = ");
 			sv_catsv(postentry, othername);
 			av_push(postav, postentry);
 		    }
@@ -517,9 +517,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 			    }
 			    else {
 				sv_catpvn(retval, name, 1);
-				sv_catpvn(retval, "{", 1);
+				sv_catpvs(retval, "{");
 				sv_catsv(retval, othername);
-				sv_catpvn(retval, "}", 1);
+				sv_catpvs(retval, "}");
 			    }
 			}
 			else
@@ -584,9 +584,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	if (!purity && maxdepth > 0 && *levelp >= maxdepth) {
 	    STRLEN vallen;
 	    const char * const valstr = SvPV(val,vallen);
-	    sv_catpvn(retval, "'", 1);
+	    sv_catpvs(retval, "'");
 	    sv_catpvn(retval, valstr, vallen);
-	    sv_catpvn(retval, "'", 1);
+	    sv_catpvs(retval, "'");
 	    return 1;
 	}
 
@@ -594,7 +594,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    STRLEN blesslen;
 	    const char * const blessstr = SvPV(bless, blesslen);
 	    sv_catpvn(retval, blessstr, blesslen);
-	    sv_catpvn(retval, "( ", 2);
+	    sv_catpvs(retval, "( ");
 	    if (indent >= 2) {
 		blesspad = apad;
 		apad = newSVsv(apad);
@@ -646,18 +646,18 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    rval = SvPV(sv_pattern, rlen);
 	    rend = rval+rlen;
 	    slash = rval;
-	    sv_catpvn(retval, "qr/", 3);
+	    sv_catpvs(retval, "qr/");
 	    for (;slash < rend; slash++) {
 	      if (*slash == '\\') { ++slash; continue; }
 	      if (*slash == '/') {    
 		sv_catpvn(retval, rval, slash-rval);
-		sv_catpvn(retval, "\\/", 2);
+		sv_catpvs(retval, "\\/");
 		rlen -= slash-rval+1;
 		rval = slash+1;
 	      }
 	    }
 	    sv_catpvn(retval, rval, rlen);
-	    sv_catpvn(retval, "/", 1);
+	    sv_catpvs(retval, "/");
 	    if (sv_flags)
 	      sv_catsv(retval, sv_flags);
 	} 
@@ -670,17 +670,17 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	) {			     /* scalar ref */
 	    SV * const namesv = newSVpvs("${");
 	    sv_catpvn(namesv, name, namelen);
-	    sv_catpvn(namesv, "}", 1);
+	    sv_catpvs(namesv, "}");
 	    if (realpack) {				     /* blessed */
-		sv_catpvn(retval, "do{\\(my $o = ", 13);
+		sv_catpvs(retval, "do{\\(my $o = ");
 		DD_dump(aTHX_ ival, SvPVX_const(namesv), SvCUR(namesv), retval, seenhv,
 			postav, levelp,	indent, pad, xpad, apad, sep, pair,
 			freezer, toaster, purity, deepcopy, quotekeys, bless,
 			maxdepth, sortkeys, use_sparse_seen_hash, useqq);
-		sv_catpvn(retval, ")}", 2);
+		sv_catpvs(retval, ")}");
 	    }						     /* plain */
 	    else {
-		sv_catpvn(retval, "\\", 1);
+		sv_catpvs(retval, "\\");
 		DD_dump(aTHX_ ival, SvPVX_const(namesv), SvCUR(namesv), retval, seenhv,
 			postav, levelp,	indent, pad, xpad, apad, sep, pair,
 			freezer, toaster, purity, deepcopy, quotekeys, bless,
@@ -691,8 +691,8 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	else if (realtype == SVt_PVGV) {		     /* glob ref */
 	    SV * const namesv = newSVpvs("*{");
 	    sv_catpvn(namesv, name, namelen);
-	    sv_catpvn(namesv, "}", 1);
-	    sv_catpvn(retval, "\\", 1);
+	    sv_catpvs(namesv, "}");
+	    sv_catpvs(retval, "\\");
 	    DD_dump(aTHX_ ival, SvPVX_const(namesv), SvCUR(namesv), retval, seenhv,
 		    postav, levelp,	indent, pad, xpad, apad, sep, pair,
 		    freezer, toaster, purity, deepcopy, quotekeys, bless,
@@ -710,11 +710,11 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    (void)strcpy(iname, name);
 	    inamelen = namelen;
 	    if (name[0] == '@') {
-		sv_catpvn(retval, "(", 1);
+		sv_catpvs(retval, "(");
 		iname[0] = '$';
 	    }
 	    else {
-		sv_catpvn(retval, "[", 1);
+		sv_catpvs(retval, "[");
 		/* omit "->" in $foo{bar}->[0], but not in ${$foo}->[0] */
 		/*if (namelen > 0
 		    && name[namelen-1] != ']' && name[namelen-1] != '}'
@@ -761,7 +761,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		if (indent >= 3) {
 		    sv_catsv(retval, totpad);
 		    sv_catsv(retval, ipad);
-		    sv_catpvn(retval, "#", 1);
+		    sv_catpvs(retval, "#");
 		    sv_catsv(retval, ixsv);
 		}
 		sv_catsv(retval, totpad);
@@ -771,7 +771,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 			freezer, toaster, purity, deepcopy, quotekeys, bless,
 			maxdepth, sortkeys, use_sparse_seen_hash, useqq);
 		if (ix < ixmax)
-		    sv_catpvn(retval, ",", 1);
+		    sv_catpvs(retval, ",");
 	    }
 	    if (ixmax >= 0) {
 		SV * const opad = sv_x(aTHX_ Nullsv, SvPVX_const(xpad), SvCUR(xpad), (*levelp)-1);
@@ -780,9 +780,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		SvREFCNT_dec(opad);
 	    }
 	    if (name[0] == '@')
-		sv_catpvn(retval, ")", 1);
+		sv_catpvs(retval, ")");
 	    else
-		sv_catpvn(retval, "]", 1);
+		sv_catpvs(retval, "]");
 	    SvREFCNT_dec(ixsv);
 	    SvREFCNT_dec(totpad);
 	    Safefree(iname);
@@ -798,11 +798,11 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	
 	    SV * const iname = newSVpvn(name, namelen);
 	    if (name[0] == '%') {
-		sv_catpvn(retval, "(", 1);
+		sv_catpvs(retval, "(");
 		(SvPVX(iname))[0] = '$';
 	    }
 	    else {
-		sv_catpvn(retval, "{", 1);
+		sv_catpvs(retval, "{");
 		/* omit "->" in $foo[0]->{bar}, but not in ${$foo}->{bar} */
 		if ((namelen > 0
 		     && name[namelen-1] != ']' && name[namelen-1] != '}')
@@ -810,16 +810,16 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		        && (name[1] == '{'
 			    || (name[0] == '\\' && name[2] == '{'))))
 		{
-		    sv_catpvn(iname, "->", 2);
+		    sv_catpvs(iname, "->");
 		}
 	    }
 	    if (name[0] == '*' && name[namelen-1] == '}' && namelen >= 8 &&
 		(instr(name+namelen-8, "{SCALAR}") ||
 		 instr(name+namelen-7, "{ARRAY}") ||
 		 instr(name+namelen-6, "{HASH}"))) {
-		sv_catpvn(iname, "->", 2);
+		sv_catpvs(iname, "->");
 	    }
-	    sv_catpvn(iname, "{", 1);
+	    sv_catpvs(iname, "{");
 	    totpad = newSVsv(sep);
 	    sv_catsv(totpad, pad);
 	    sv_catsv(totpad, apad);
@@ -894,7 +894,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
                }
 
 		if (i)
-		    sv_catpvn(retval, ",", 1);
+		    sv_catpvs(retval, ",");
 
 		if (sortkeys) {
 		    char *key;
@@ -961,7 +961,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		}
                 sname = newSVsv(iname);
                 sv_catpvn(sname, nkey, nlen);
-                sv_catpvn(sname, "}", 1);
+                sv_catpvs(sname, "}");
 
 		sv_catsv(retval, pair);
 		if (indent >= 2) {
@@ -994,14 +994,14 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		SvREFCNT_dec(opad);
 	    }
 	    if (name[0] == '%')
-		sv_catpvn(retval, ")", 1);
+		sv_catpvs(retval, ")");
 	    else
-		sv_catpvn(retval, "}", 1);
+		sv_catpvs(retval, "}");
 	    SvREFCNT_dec(iname);
 	    SvREFCNT_dec(totpad);
 	}
 	else if (realtype == SVt_PVCV) {
-	    sv_catpvn(retval, "sub { \"DUMMY\" }", 15);
+	    sv_catpvs(retval, "sub { \"DUMMY\" }");
 	    if (purity)
 		warn("Encountered CODE ref, using dummy placeholder");
 	}
@@ -1017,7 +1017,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		SvREFCNT_dec(apad);
 		apad = blesspad;
 	    }
-	    sv_catpvn(retval, ", '", 3);
+	    sv_catpvs(retval, ", '");
 
 	    plen = strlen(realpack);
 	    pticks = num_q(realpack, plen);
@@ -1036,11 +1036,11 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    else {
 	        sv_catpvn(retval, realpack, strlen(realpack));
 	    }
-	    sv_catpvn(retval, "' )", 3);
+	    sv_catpvs(retval, "' )");
 	    if (toaster && SvPOK(toaster) && SvCUR(toaster)) {
-		sv_catpvn(retval, "->", 2);
+		sv_catpvs(retval, "->");
 		sv_catsv(retval, toaster);
-		sv_catpvn(retval, "()", 2);
+		sv_catpvs(retval, "()");
 	    }
 	}
 	SvREFCNT_dec(ipad);
@@ -1065,9 +1065,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 		if ((svp = av_fetch(seenentry, 0, FALSE)) && (othername = *svp)
 		    && (svp = av_fetch(seenentry, 2, FALSE)) && *svp && SvIV(*svp) > 0)
 		{
-		    sv_catpvn(retval, "${", 2);
+		    sv_catpvs(retval, "${");
 		    sv_catsv(retval, othername);
-		    sv_catpvn(retval, "}", 1);
+		    sv_catpvs(retval, "}");
 		    return 1;
 		}
 	    }
@@ -1178,7 +1178,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 			
 			sv_setsv(nname, postentry);
 			sv_catpvn(nname, entries[j], sizes[j]);
-			sv_catpvn(postentry, " = ", 3);
+			sv_catpvs(postentry, " = ");
 			av_push(postav, postentry);
 			e = newRV_inc(e);
 			
@@ -1200,7 +1200,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv,
 	    }
 	}
 	else if (val == &PL_sv_undef || !SvOK(val)) {
-	    sv_catpvn(retval, "undef", 5);
+	    sv_catpvs(retval, "undef");
 	}
 #ifdef SvVOK
 	else if (SvMAGICAL(val) && (mg = mg_find(val, 'V'))) {
@@ -1455,7 +1455,7 @@ Data_Dumper_Dumpxs(href, ...)
 		    if (postlen >= 0 || !terse) {
 			sv_insert(valstr, 0, 0, " = ", 3);
 			sv_insert(valstr, 0, 0, SvPVX_const(name), SvCUR(name));
-			sv_catpvn(valstr, ";", 1);
+			sv_catpvs(valstr, ";");
 		    }
 		    sv_catsv(retval, pad);
 		    sv_catsv(retval, valstr);
@@ -1469,13 +1469,13 @@ Data_Dumper_Dumpxs(href, ...)
 			    if (svp && (elem = *svp)) {
 				sv_catsv(retval, elem);
 				if (i < postlen) {
-				    sv_catpvn(retval, ";", 1);
+				    sv_catpvs(retval, ";");
 				    sv_catsv(retval, sep);
 				    sv_catsv(retval, pad);
 				}
 			    }
 			}
-			sv_catpvn(retval, ";", 1);
+			sv_catpvs(retval, ";");
 			    sv_catsv(retval, sep);
 		    }
 		    sv_setpvn(valstr, "", 0);
diff --git a/pp_ctl.c b/pp_ctl.c
index 6a619ce..b559c5a 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -3649,7 +3649,7 @@ S_doopen_pm(pTHX_ SV *name)
 	Stat_t pmcstat;
 
 	SvSetSV_nosteal(pmcsv,name);
-	sv_catpvn(pmcsv, "c", 1);
+	sv_catpvs(pmcsv, "c");
 
 	if (PerlLIO_stat(SvPV_nolen_const(pmcsv), &pmcstat) >= 0)
 	    return check_type_and_open(pmcsv);
@@ -4078,7 +4078,7 @@ PP(pp_require)
 			sv_catpv(msg, " (you may need to install the ");
 			for (c = name; c < e; c++) {
 			    if (*c == '/') {
-				sv_catpvn(msg, "::", 2);
+				sv_catpvs(msg, "::");
 			    }
 			    else {
 				sv_catpvn(msg, c, 1);
diff --git a/win32/win32.c b/win32/win32.c
index b601452..26d419e 100644
--- a/win32/win32.c
+++ b/win32/win32.c
@@ -360,7 +360,7 @@ get_emd_part(SV **prev_pathp, STRLEN *const len, char *trailing_path, ...)
 	if (!*prev_pathp)
 	    *prev_pathp = sv_2mortal(newSVpvs(""));
 	else if (SvPVX(*prev_pathp))
-	    sv_catpvn(*prev_pathp, ";", 1);
+	    sv_catpvs(*prev_pathp, ";");
 	sv_catpv(*prev_pathp, mod_name);
 	if(len)
 	    *len = SvCUR(*prev_pathp);
@@ -418,7 +418,7 @@ win32_get_xlib(const char *pl, const char *xlib, const char *libname,
 	sv1 = sv2;
     } else if (sv2) {
         dTHX;
-	sv_catpvn(sv1, ";", 1);
+	sv_catpv(sv1, ";");
 	sv_catsv(sv1, sv2);
     }
 
diff --git a/win32/wince.c b/win32/wince.c
index bbe8d31..271df2b 100644
--- a/win32/wince.c
+++ b/win32/wince.c
@@ -227,7 +227,7 @@ get_emd_part(SV **prev_pathp, STRLEN *const len, char *trailing_path, ...)
 	dTHX;
 	if (!*prev_pathp)
 	    *prev_pathp = sv_2mortal(newSVpvs(""));
-	sv_catpvn(*prev_pathp, ";", 1);
+	sv_catpvs(*prev_pathp, ";");
 	sv_catpv(*prev_pathp, mod_name);
 	if(len)
 	    *len = SvCUR(*prev_pathp);
@@ -286,7 +286,7 @@ win32_get_xlib(const char *pl, const char *xlib, const char *libname,
     if (!sv1) {
 	sv1 = sv2;
     } else if (sv2) {
-	sv_catpvn(sv1, ";", 1);
+	sv_catpvs(sv1, ";");
 	sv_catsv(sv1, sv2);
     }
 
-- 
2.0.0

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 12, 2014

From @tonycoz

On Mon Jun 09 11​:26​:41 2014, ilmari wrote​:

Oops, the previous patch had a spurious s/pvn/pvs/ for a non-constant
item, so failed to compile. Here's an updated version which builds and
passes all tests.

Thanks, applied as c2b90b6 and 46e2868.

The change to Data​::Dumper did fix a bug, the following​:

./perl -Ilib -MData​::Dumper -le 'sub x {} print Data​::Dumper->new([\&x, \\&x], ["*b", "c"])->Dump'

would produce the incorrect result you predicted. The pure perl code produced the correct result.

I added a test for the correct behaviour in 4662a05.

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 12, 2014

The RT System itself - Status changed from 'new' to 'open'

@p5pRT p5pRT closed this Jun 12, 2014
@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 12, 2014

@tonycoz - Status changed from 'open' to 'resolved'

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 12, 2014

From @ilmari

"Tony Cook via RT" <perlbug-followup@​perl.org> writes​:

The change to Data​::Dumper did fix a bug, the following​:

./perl -Ilib -MData​::Dumper -le 'sub x {} print Data​::Dumper->new([\&x, \\&x], ["*b", "c"])->Dump'

would produce the incorrect result you predicted. The pure perl code
produced the correct result.

I added a test for the correct behaviour in
4662a05.

Shouldn't that test be un-TODOed now that it's fixed?

--
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article. - Calle Dybedahl

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 24, 2014

From @tonycoz

On Thu Jun 12 00​:50​:41 2014, ilmari wrote​:

"Tony Cook via RT" <perlbug-followup@​perl.org> writes​:

The change to Data​::Dumper did fix a bug, the following​:

./perl -Ilib -MData​::Dumper -le 'sub x {} print Data​::Dumper-

new([\&x, \\&x], ["*b", "c"])->Dump'

would produce the incorrect result you predicted. The pure perl code
produced the correct result.

I added a test for the correct behaviour in
4662a05.

Shouldn't that test be un-TODOed now that it's fixed?

Removed the $TODO in b658274.

This didn't actually make the test TODO, since dumper.t uses its own test framework which ignores $TODO.

Tony

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.