Skip to content

Commit

Permalink
builtin::trim: ensure TARG is properly reset
Browse files Browse the repository at this point in the history
refactor a lot of custom "set SV to a string code" away to
sv_setpvn(), this:

 - fixed the original problem reported for Perl#22784, where TARG wasn't
   being reset properly and contained a cached numeric version of the
   result from the previous call.

 - removed some never executed code, since builtin::trim is only XS
   and is not an OP with the TARGLEX optimization

 - fixes a possible problem if the result of the first call to trim()
   is COWed.

This does slightly change the taint behaviour, rather than making TARG
tainted iff source is tainted, it changes to the behaviour of the rest
of perl, making TARG tainted if any tainted input is seen in the
current expression.

See thr PR Perl#22788 for some discussion on how we got here.

Fixes Perl#22784
  • Loading branch information
tonycoz committed Dec 1, 2024
1 parent 342b57d commit 35e27db
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 27 deletions.
32 changes: 6 additions & 26 deletions builtin.c
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ XS(XS_builtin_trim)
SV *source = TOPs;
STRLEN len;
const U8 *start;
SV *dest;

SvGETMAGIC(source);

Expand Down Expand Up @@ -273,33 +272,14 @@ XS(XS_builtin_trim)
}
}

dest = TARG;
sv_setpvn(TARG, (const char *)start, len);

if (SvPOK(dest) && (dest == source)) {
sv_chop(dest, (const char *)start);
SvCUR_set(dest, len);
}
else {
SvUPGRADE(dest, SVt_PV);
SvGROW(dest, len + 1);

Copy(start, SvPVX(dest), len, U8);
SvPVX(dest)[len] = '\0';
SvPOK_on(dest);
SvCUR_set(dest, len);

if (DO_UTF8(source))
SvUTF8_on(dest);
else
SvUTF8_off(dest);

if (SvTAINTED(source))
SvTAINT(dest);
}

SvSETMAGIC(dest);
if (DO_UTF8(source))
SvUTF8_on(TARG);
else
SvUTF8_off(TARG);

SETs(dest);
SETTARG;

XSRETURN(1);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/builtin.pm
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package builtin 0.016;
package builtin 0.017;

use v5.40;

Expand Down
8 changes: 8 additions & 0 deletions lib/builtin.t
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,14 @@ EOS
}
}

# github #22784
{
use builtin qw( trim );
sub f { 0+trim($_[0]) }
is(f(4), 4, "populate TARG.iv");
is(f(123), 123, "check TARG.IOK is reset properly");
}

# vim: tabstop=4 shiftwidth=4 expandtab autoindent softtabstop=4

done_testing();

0 comments on commit 35e27db

Please sign in to comment.