Re: [PATCH] Guard against killing unrelated processes in amd64-disp-step.exp

[Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>]

Hi Andrew,

>> diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.exp b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
>> --- a/gdb/testsuite/gdb.arch/amd64-disp-step.exp
>> +++ b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
>> @@ -222,7 +222,10 @@ proc rip_test { reg test_start_label tes
>>  	    # If we use 'signal' to send the signal GDB doesn't actually do
>>  	    # the displaced step, but instead just delivers the signal.
>>  	    set inferior_pid [get_inferior_pid]
>> -	    remote_exec target "kill -ALRM $inferior_pid"
>> +	    # Ensure that $inferior_pid refers to a single process.
>> +	    if {$inferior_pid > 0} {
>> +		remote_exec target "kill -ALRM $inferior_pid"
>> +	    }
>
> Does this not hide the fact that the test is no longer doing what it
> expected?

it does.  However, the results for this particular test were so bad
already that I didn't think about one or more FAILs here.

> I'm fine with the 'if {$inferior_pid > 0}' being added to ensure we
> don't signal some random process(es), but I think we should probably add
> something like:
>
>   gdb_assert {[expr $inferior_pid > 0]} \
>     "check for a sane inferior pid"
>   if {$inferior_pid > 0} {
>     remote_exec target "kill -ALRM $inferior_pid"
>   }
>
> This way you will still see a FAIL.

True, but you will also see quite a bunch of PASSes in the working case
that tell you nothing.  Seems like unnecessary noise to me.  Isn't there
another way to convey the failure info without that noise?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University