[PATCH] Fix kill of processes created by win32_create_inferior

[PATCH] Fix kill of processes created by win32_create_inferior

[Hannes Domani via gdb-patches]

handle_v_kill uses signal_pid because win32 doesn't support multi-process.

gdbserver/ChangeLog:

2020-02-08  Hannes Domani  <ssbssa@yahoo.de>

	* win32-low.c (win32_create_inferior): Set signal_pid.
---
 gdbserver/win32-low.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdbserver/win32-low.c b/gdbserver/win32-low.c
index 9d0343788f..557c90d97c 100644
--- a/gdbserver/win32-low.c
+++ b/gdbserver/win32-low.c
@@ -709,6 +709,9 @@ win32_create_inferior (const char *program,
      (assuming success).  */
   cs.last_ptid = win32_wait (ptid_t (current_process_id), &cs.last_status, 0);
 
+  /* Necessary for handle_v_kill.  */
+  signal_pid = current_process_id;
+
   return current_process_id;
 }
 
-- 
2.25.0

Re: [PATCH] Fix kill of processes created by win32_create_inferior

[Luis Machado]

On 2/8/20 3:25 PM, Hannes Domani via gdb-patches wrote:
> handle_v_kill uses signal_pid because win32 doesn't support multi-process.
> 
> gdbserver/ChangeLog:
> 
> 2020-02-08  Hannes Domani  <ssbssa@yahoo.de>
> 
> 	* win32-low.c (win32_create_inferior): Set signal_pid.
> ---
>   gdbserver/win32-low.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/gdbserver/win32-low.c b/gdbserver/win32-low.c
> index 9d0343788f..557c90d97c 100644
> --- a/gdbserver/win32-low.c
> +++ b/gdbserver/win32-low.c
> @@ -709,6 +709,9 @@ win32_create_inferior (const char *program,
>        (assuming success).  */
>     cs.last_ptid = win32_wait (ptid_t (current_process_id), &cs.last_status, 0);
>   
> +  /* Necessary for handle_v_kill.  */
> +  signal_pid = current_process_id;
> +
>     return current_process_id;
>   }
>   
> 

Thanks. This one looks OK to me.

It would be nice to augment the description with what situation this 
fixes. I suppose it doesn't kill the process properly?

Re: [PATCH] Fix kill of processes created by win32_create_inferior

[Simon Marchi]

On 2020-02-10 5:33 p.m., Luis Machado wrote:
> On 2/8/20 3:25 PM, Hannes Domani via gdb-patches wrote:
>> handle_v_kill uses signal_pid because win32 doesn't support multi-process.
>>
>> gdbserver/ChangeLog:
>>
>> 2020-02-08  Hannes Domani  <ssbssa@yahoo.de>
>>
>> 	* win32-low.c (win32_create_inferior): Set signal_pid.
>> ---
>>   gdbserver/win32-low.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/gdbserver/win32-low.c b/gdbserver/win32-low.c
>> index 9d0343788f..557c90d97c 100644
>> --- a/gdbserver/win32-low.c
>> +++ b/gdbserver/win32-low.c
>> @@ -709,6 +709,9 @@ win32_create_inferior (const char *program,
>>        (assuming success).  */
>>     cs.last_ptid = win32_wait (ptid_t (current_process_id), &cs.last_status, 0);
>>   
>> +  /* Necessary for handle_v_kill.  */
>> +  signal_pid = current_process_id;
>> +
>>     return current_process_id;
>>   }
>>   
>>
> 
> Thanks. This one looks OK to me.
> 
> It would be nice to augment the description with what situation this 
> fixes. I suppose it doesn't kill the process properly?

Agreed.  The patch looks ok, as far as I understand, but it would be good to
explain in the commit message what's the bug, how you provoke it, how the new
behavior differs from the previous behavior.

Simon

Re: [PATCH] Fix kill of processes created by win32_create_inferior

[Simon Marchi]

On 2020-02-11 11:18 p.m., Simon Marchi wrote:
> On 2020-02-10 5:33 p.m., Luis Machado wrote:
>> On 2/8/20 3:25 PM, Hannes Domani via gdb-patches wrote:
>>> handle_v_kill uses signal_pid because win32 doesn't support multi-process.
>>>
>>> gdbserver/ChangeLog:
>>>
>>> 2020-02-08  Hannes Domani  <ssbssa@yahoo.de>
>>>
>>> 	* win32-low.c (win32_create_inferior): Set signal_pid.
>>> ---
>>>   gdbserver/win32-low.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/gdbserver/win32-low.c b/gdbserver/win32-low.c
>>> index 9d0343788f..557c90d97c 100644
>>> --- a/gdbserver/win32-low.c
>>> +++ b/gdbserver/win32-low.c
>>> @@ -709,6 +709,9 @@ win32_create_inferior (const char *program,
>>>        (assuming success).  */
>>>     cs.last_ptid = win32_wait (ptid_t (current_process_id), &cs.last_status, 0);
>>>   
>>> +  /* Necessary for handle_v_kill.  */
>>> +  signal_pid = current_process_id;
>>> +
>>>     return current_process_id;
>>>   }
>>>   
>>>
>>
>> Thanks. This one looks OK to me.
>>
>> It would be nice to augment the description with what situation this 
>> fixes. I suppose it doesn't kill the process properly?
> 
> Agreed.  The patch looks ok, as far as I understand, but it would be good to
> explain in the commit message what's the bug, how you provoke it, how the new
> behavior differs from the previous behavior.
> 
> Simon
> 

Sorry, I just saw that you have already posted a v2, I'll at it now.

Simon