Re: [PATCH] Double magic with substr

On Mon, Mar 31, 2008 at 07:05:44PM +0200, Vincent Pit wrote:

> I kept a symbol to the old Perl_sv_insert so that modules built against 
> it can still link, but I don't know if this has to be expected or if 
> there's a better way to achieve it.

I think that the "better" way is to define a macro that provides the old
name as a wrapper to the new name, and move the implementation of the old
name to mathoms.c. Somewhat like the appended change.

Nicholas Clark

Change 33627 by nicholas@nicholas-saigo on 2008/04/01 19:59:54

	Define sv_insert() as a wrapper to sv_insert_flags(), and move
	Perl_sv_insert() to mathoms.c

Affected files ...

... //depot/perl/embed.fnc#608 edit
... //depot/perl/embed.h#755 edit
... //depot/perl/mathoms.c#87 edit
... //depot/perl/mg.c#524 edit
... //depot/perl/proto.h#943 edit
... //depot/perl/sv.c#1534 edit
... //depot/perl/sv.h#346 edit

Differences ...

==== //depot/perl/embed.fnc#608 (text) ====

@@ -882,8 +882,9 @@
 Apd	|char*	|sv_gets	|NN SV *const sv|NN PerlIO *const fp|I32 append
 Apd	|char*	|sv_grow	|NN SV *const sv|STRLEN newlen
 Apd	|void	|sv_inc		|NULLOK SV *const sv
-Apd	|void	|sv_insert	|NN SV *const bigstr|const STRLEN offset|const STRLEN len \
-				|NN const char *const little|const STRLEN littlelen
+Amdb	|void	|sv_insert	|NN SV *const bigstr|const STRLEN offset \
+				|const STRLEN len|NN const char *const little \
+				|const STRLEN littlelen
 Apd	|void	|sv_insert_flags|NN SV *const bigstr|const STRLEN offset|const STRLEN len \
 				|NN const char *const little|const STRLEN littlelen|const U32 flags
 Apd	|int	|sv_isa		|NULLOK SV* sv|NN const char *const name

==== //depot/perl/embed.h#755 (text+w) ====

@@ -885,7 +885,6 @@
 #define sv_gets			Perl_sv_gets
 #define sv_grow			Perl_sv_grow
 #define sv_inc			Perl_sv_inc
-#define sv_insert		Perl_sv_insert
 #define sv_insert_flags		Perl_sv_insert_flags
 #define sv_isa			Perl_sv_isa
 #define sv_isobject		Perl_sv_isobject
@@ -3188,7 +3187,6 @@
 #define sv_gets(a,b,c)		Perl_sv_gets(aTHX_ a,b,c)
 #define sv_grow(a,b)		Perl_sv_grow(aTHX_ a,b)
 #define sv_inc(a)		Perl_sv_inc(aTHX_ a)
-#define sv_insert(a,b,c,d,e)	Perl_sv_insert(aTHX_ a,b,c,d,e)
 #define sv_insert_flags(a,b,c,d,e,f)	Perl_sv_insert_flags(aTHX_ a,b,c,d,e,f)
 #define sv_isa(a,b)		Perl_sv_isa(aTHX_ a,b)
 #define sv_isobject(a)		Perl_sv_isobject(aTHX_ a)

==== //depot/perl/mathoms.c#87 (text) ====

@@ -1461,6 +1461,14 @@
     return hv;
 }
 
+void
+Perl_sv_insert(pTHX_ SV *const bigstr, const STRLEN offset, const STRLEN len, 
+              const char *const little, const STRLEN littlelen)
+{
+    PERL_ARGS_ASSERT_SV_INSERT;
+    sv_insert_flags(bigstr, offset, len, little, littlelen, SV_GMAGIC);
+}
+
 #endif /* NO_MATHOMS */
 
 /*

==== //depot/perl/mg.c#524 (text) ====

@@ -1560,7 +1560,8 @@
 	 * tell whether HINT_STRICT_REFS is in force or not.
 	 */
 	if (!strchr(s,':') && !strchr(s,'\''))
-	    Perl_sv_insert(aTHX_ sv, 0, 0, STR_WITH_LEN("main::"));
+	    Perl_sv_insert_flags(aTHX_ sv, 0, 0, STR_WITH_LEN("main::"),
+				 SV_GMAGIC);
 	if (i)
 	    (void)rsignal(i, PL_csighandlerp);
 	else

==== //depot/perl/proto.h#943 (text+w) ====

@@ -3179,9 +3179,9 @@
 	assert(sv)
 
 PERL_CALLCONV void	Perl_sv_inc(pTHX_ SV *const sv);
-PERL_CALLCONV void	Perl_sv_insert(pTHX_ SV *const bigstr, const STRLEN offset, const STRLEN len, const char *const little, const STRLEN littlelen)
+/* PERL_CALLCONV void	Perl_sv_insert(pTHX_ SV *const bigstr, const STRLEN offset, const STRLEN len, const char *const little, const STRLEN littlelen)
 			__attribute__nonnull__(pTHX_1)
-			__attribute__nonnull__(pTHX_4);
+			__attribute__nonnull__(pTHX_4); */
 #define PERL_ARGS_ASSERT_SV_INSERT	\
 	assert(bigstr); assert(little)
 

==== //depot/perl/sv.c#1534 (text) ====

@@ -5142,18 +5142,6 @@
 Inserts a string at the specified offset/length within the SV. Similar to
 the Perl substr() function. Handles get magic.
 
-=cut
-*/
-
-void
-Perl_sv_insert(pTHX_ SV *const bigstr, const STRLEN offset, const STRLEN len, 
-              const char *const little, const STRLEN littlelen)
-{
-    PERL_ARGS_ASSERT_SV_INSERT;
-    sv_insert_flags(bigstr, offset, len, little, littlelen, SV_GMAGIC);
-}
-
-/*
 =for apidoc sv_insert_flags
 
 Same as C<sv_insert>, but the extra C<flags> are passed the C<SvPV_force_flags> that applies to C<bigstr>.
@@ -12325,8 +12313,10 @@
 	*SvPVX(name) = '$';
 	Perl_sv_catpvf(aTHX_ name, "[%"IVdf"]", (IV)aindex);
     }
-    else if (subscript_type == FUV_SUBSCRIPT_WITHIN)
-	Perl_sv_insert(aTHX_ name, 0, 0,  STR_WITH_LEN("within "));
+    else if (subscript_type == FUV_SUBSCRIPT_WITHIN) {
+	/* We know that name has no magic, so can use 0 instead of SV_GMAGIC */
+	Perl_sv_insert_flags(aTHX_ name, 0, 0,  STR_WITH_LEN("within "), 0);
+    }
 
     return name;
 }

==== //depot/perl/sv.h#346 (text) ====

@@ -1778,6 +1778,9 @@
 #define sv_utf8_upgrade(sv) sv_utf8_upgrade_flags(sv, SV_GMAGIC)
 #define sv_2iv(sv) sv_2iv_flags(sv, SV_GMAGIC)
 #define sv_2uv(sv) sv_2uv_flags(sv, SV_GMAGIC)
+#define sv_insert(bigstr, offset, len, little, littlelen)		\
+	Perl_sv_insert_flags(aTHX_ (bigstr),(offset), (len), (little),	\
+			     (littlelen), SV_GMAGIC)
 
 /* Should be named SvCatPVN_utf8_upgrade? */
 #define sv_catpvn_utf8_upgrade(dsv, sstr, slen, nsv)	\

• [PATCH] Double magic with substr by Vincent Pit
• Re: [PATCH] Double magic with substr by Nicholas Clark
• Re: [PATCH] Double magic with substr by Vincent Pit
• Re: [PATCH] Double magic with substr by Nicholas Clark
• Re: [PATCH] Double magic with substr by Rafael Garcia-Suarez