Re: [PATCH v5] Enable 'set print inferior-events' and improve detach/fork/kill/exit messages

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Wednesday, April 11 2018, Pedro Alves wrote:

> On 04/11/2018 07:46 PM, Sergio Durigan Junior wrote:
>> Changes from v4:
>> 
>> - Fix race on gdb.threads/process-dies-while-detaching-1-detach.exp.
>
> I don't think that testcase exists.  :-)  Anyway, just remember
> to remove the "Changes from" section before pushing that then
> it's moot.

That's true, wrong copy&paste.

>> 
>> - Fix some wrong regexps when expecting for the inferior to be killed.
>
> Hmm, you didn't address the comments to the gdb code itself.

Sigh.  Sorry about that.  I'll do that now.

Thanks,

>
>> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
>> index d43e7f202d..1f724fbf23 100644
>> --- a/gdb/infcmd.c
>> +++ b/gdb/infcmd.c
>> @@ -2595,8 +2595,16 @@ kill_command (const char *arg, int from_tty)
>>      error (_("The program is not being run."));
>>    if (!query (_("Kill the program being debugged? ")))
>>      error (_("Not confirmed."));
>> +
>> +  const char *pid_str = target_pid_to_str (inferior_ptid);
>> +  int infnum = current_inferior ()->num;
>> +
>>    target_kill ();
>>  
>> +  if (print_inferior_events)
>> +    printf_unfiltered (_("[Inferior %d (%s) has been killed]\n"),
>> +		       infnum, pid_str);
>> +
>
> My previous comment still applies here:
>
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  This still seem risky -- Target backends use target_pid_to_str inside
>  target_kill, e.g., when logging is enabled.  E.g., 
>
>  linux_nat_kill -> -> stop_callback -> target_pid_to_str
>
>  ISTM a deep copy like:
>
>    std::string pid_str = target_pid_to_str (inferior_ptid);
>
>  would be safer/better.
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>> @@ -123,7 +122,8 @@ add_inferior (int pid)
>>    struct inferior *inf = add_inferior_silent (pid);
>>  
>>    if (print_inferior_events)
>> -    printf_unfiltered (_("[New inferior %d]\n"), pid);
>> +    printf_unfiltered (_("[New inferior %d (process %d)]\n"),
>> +		       inf->num, pid);
>
> Likewise.
>
>> @@ -266,7 +263,8 @@ detach_inferior (inferior *inf)
>>    exit_inferior_1 (inf, 0);
>>  
>>    if (print_inferior_events)
>> -    printf_unfiltered (_("[Inferior %d detached]\n"), pid);
>> +    printf_unfiltered (_("[Inferior %d (process %d) detached]\n"),
>> +		       inf->num, pid);
>>  }
>>  
>
> Likewise?
>
>> -      if (info_verbose || debug_infrun)
>> +      if (print_inferior_events)
>>  	{
>> +	  gdb::unique_xmalloc_ptr<char>
>> +	    parent_pid (xstrdup (target_pid_to_str (parent_ptid)));
>> +	  gdb::unique_xmalloc_ptr<char>
>> +	    child_pid (xstrdup (target_pid_to_str (child_ptid)));
>> +
>
> Likewise?
>
> /me stops looking further.
>
> Looks like you missed handling the comments for the gdb code.  
>
> Try again? :-)
>
> Thanks,
> Pedro Alves

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/