develooper Front page | perl.perl5.porters | Postings from June 2010

Re: [perl #75812] kill $1 broken in 5.12.1

Thread Next
From:
Todd Rinaldo
Date:
June 20, 2010 05:56
Subject:
Re: [perl #75812] kill $1 broken in 5.12.1
Message ID:
5118C9DB-B18C-41A0-B5A2-173E4BD45369@cpanel.net

On Jun 17, 2010, at 4:16 AM, Tony Cook via RT wrote:

> On Wed Jun 16 13:08:00 2010, toddr@cpanel.net wrote:
>> We've discovered that this snippet will break in perl 5.12
>> 
>> my $pid = "12345  ";
>> if($pid =~ m/(\d+)/) {
>> 	kill 9, $1;
>> }
>> 
>> it dies with this message:
>> Can't kill a non-numeric process ID at test.pl line 7.
>> 
>> 
>> The reason appears to be a result of this commit.
>> 
> http://perl5.git.perl.org/perl.git/commitdiff/e2c0f81f627951896aca833460887e6e8f20aba6?hp=32878f30ba1216461c8932946f0868cda9920d62
>> 
>> Using Devel::Peek, I'm guessing the problem is that IV doesn't get set
>> on a $1 variable?
>> 
>> For some reason I can't understand,  I can make this problem go away
>> if I do 'use 5.12.1' at the beginning of my code. $1+0 also forces
>> kill to work.
> 
> $1 is magic, so until SvGETMAGIC() is called it has no flags.
> 
> Patch calls SvGETMAGIC() before checking the flags, and uses the no
> magic variant of SvIV() avoid to double magic.
> 
> looks_like_number() can also call SvGETMAGIC() via SvPV_const(), but
> that's a different problem.
> 
> Tony
> From 322ed3e750982361473addd7d45edcbace447752 Mon Sep 17 00:00:00 2001
> From: Tony Cook <tony@develop-help.com>
> Date: Thu, 17 Jun 2010 18:58:04 +1000
> Subject: [PATCH] RT #75812: apply get magic before checking flags, PVX
> 
> The code was checking flags with applying any get magic, so when a
> match was doing putting a numeric string into $1, none of the flags
> checked were set, so producing the "non-numeric process ID" error.
> ---
> doio.c       |    9 ++++++---
> t/op/kill0.t |    9 ++++++++-
> 2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/doio.c b/doio.c
> index 98861af..06f2d3d 100644
> --- a/doio.c
> +++ b/doio.c
> @@ -1721,9 +1721,10 @@ nothing in the core.
> 	    while (++mark <= sp) {
> 		I32 proc;
> 		register unsigned long int __vmssts;
> +		SvGETMAGIC(*mark);
> 		if (!(SvIOK(*mark) || SvNOK(*mark) || looks_like_number(*mark)))
> 		    Perl_croak(aTHX_ "Can't kill a non-numeric process ID");
> -		proc = SvIV(*mark);
> +		proc = SvIV_nomg(*mark);
> 		APPLY_TAINT_PROPER();
> 		if (!((__vmssts = sys$delprc(&proc,0)) & 1)) {
> 		    tot--;
> @@ -1748,9 +1749,10 @@ nothing in the core.
> 	    val = -val;
> 	    while (++mark <= sp) {
> 		I32 proc;
> +		SvGETMAGIC(*mark);
> 		if (!(SvIOK(*mark) || SvNOK(*mark) || looks_like_number(*mark)))
> 		    Perl_croak(aTHX_ "Can't kill a non-numeric process ID");
> -		proc = SvIV(*mark);
> +		proc = SvIV_nomg(*mark);
> 		APPLY_TAINT_PROPER();
> #ifdef HAS_KILLPG
> 		if (PerlProc_killpg(proc,val))	/* BSD */
> @@ -1763,9 +1765,10 @@ nothing in the core.
> 	else {
> 	    while (++mark <= sp) {
> 		I32 proc;
> +		SvGETMAGIC(*mark);
> 		if (!(SvIOK(*mark) || SvNOK(*mark) || looks_like_number(*mark)))
> 		    Perl_croak(aTHX_ "Can't kill a non-numeric process ID");
> -		proc = SvIV(*mark);
> +		proc = SvIV_nomg(*mark);
> 		APPLY_TAINT_PROPER();
> 		if (PerlProc_kill(proc, val))
> 		    tot--;
> diff --git a/t/op/kill0.t b/t/op/kill0.t
> index eadf15d..d3ef8f7 100644
> --- a/t/op/kill0.t
> +++ b/t/op/kill0.t
> @@ -14,7 +14,7 @@ BEGIN {
> 
> use strict;
> 
> -plan tests => 5;
> +plan tests => 6;
> 
> ok( kill(0, $$), 'kill(0, $pid) returns true if $pid exists' );
> 
> @@ -43,3 +43,10 @@ for my $case ( @bad_pids ) {
>   like( $@, qr/^Can't kill a non-numeric process ID/, "dies killing $name pid");
> }
> 
> +# Verify that killing a magic variable containing a number doesn't
> +# trigger the above
> +{
> +  my $x = $$ . " ";
> +  $x =~ /(\d+)/;
> +  ok(eval { kill 0, $1 }, "can kill a number string in a magic variable");
> +}
> -- 
> 1.5.6.5
> 

Short of fixing looks_like_number, Tony's patch with tests looks good. I didn't see any real objections to the patch in the thread. Could this be pushed into blead?


Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About