From f5f6b4773901bb01cbdbfff1d09452d56d6b8f91 Mon Sep 17 00:00:00 2001 From: Richard Leach Date: Fri, 10 Feb 2023 20:19:56 +0000 Subject: [PATCH 1/2] pp_substr: no need to redo work when cloning the replacement string Currently, when copying the replacement string in preparation for `sv_utf8_upgrade()`, `pp_substr` calls: repl_sv_copy = newSVsv(repl_sv); However, `repl_sv` was previously checked for GMAGIC, coerced to a string if necessary, and the char * and length obtained by: repl = SvPV_const(repl_sv, repl_len); We should be able to produce a copy more directly by just doing: repl_sv_copy = newSVpvn(repl, repl_len); --- pp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pp.c b/pp.c index 98edc8f37b92..9beda2d99a83 100644 --- a/pp.c +++ b/pp.c @@ -3434,7 +3434,7 @@ PP(pp_substr) SV* repl_sv_copy = NULL; if (repl_need_utf8_upgrade) { - repl_sv_copy = newSVsv(repl_sv); + repl_sv_copy = newSVpvn(repl, repl_len); sv_utf8_upgrade(repl_sv_copy); repl = SvPV_const(repl_sv_copy, repl_len); } From 55927f5bf804c4e414f5ba7b146ba8911dad12c1 Mon Sep 17 00:00:00 2001 From: Richard Leach Date: Fri, 10 Jan 2025 23:34:09 +0000 Subject: [PATCH 2/2] pp_substr: checking SvOK() is superfluous after SvPV_force_nomg() When replacing some characters in pp_substr, there should be no need to check `SvOK(sv)`, since previous calls to `SvPV_force_nomg(sv)` and (if needed) `sv_utf8_upgrade_nomg(sv);` should always return a `sv` that is `SvOK`. --- pp.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pp.c b/pp.c index 9beda2d99a83..65d0287e35cf 100644 --- a/pp.c +++ b/pp.c @@ -3438,8 +3438,11 @@ PP(pp_substr) sv_utf8_upgrade(repl_sv_copy); repl = SvPV_const(repl_sv_copy, repl_len); } - if (!SvOK(sv)) - SvPVCLEAR(sv); + + /* The earlier SvPV_force_nomg(sv, curlen) should have ensured + * that sv is SvOK, even if it wasn't beforehand. */ + assert(SvOK(sv)); + sv_insert_flags(sv, byte_pos, byte_len, repl, repl_len, 0); SvREFCNT_dec(repl_sv_copy); }