Re: Making mi -add-inferior command consistent with ci

[Muhammad Umair Sair <umair_sair@hotmail.com>]

On 2/22/22 7:08 PM, Andrew Burgess wrote:
> Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>>  From 3729f798d1f75a74eefe0c240d8c4c9849dac308 Mon Sep 17 00:00:00 2001
>> From: Umair Sair <umair_sair@hotmail.com>
>> Date: Tue, 22 Feb 2022 01:40:06 +0500
>> Subject: [PATCH] Making mi -add-inferior command consistent with ci
>>
>> With multi-target debugging support in gdb, add-inferior command without
>> arguments in console interpreter adds a new inferior and sets the connection
>> of previously selected inferior as the connection of new inferior as well.
>> But the same command for machine interpreter is not setting the connection
>> for new inferior. This commit fixes this issue and makes the console and
>> machine interpreter behavior consistent for add-inferior command.
> 
> We're usually pretty conservative about changing MI behaviour in non
> backward compatible ways.  I'm not entirely sure that this change wont
> cause problems for an existing f/e that is using -add-inferior?
> 
> Maybe I'm worrying about nothing?  I'm open to being convinced.

The older change broke it in non backward compatibility way and this 
commit is fixing it. I faced issues compared to older versions of gdb 
and on investigation I found this issue. Following is the commit where 
we supported multi-target.

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2

> 
> Another option would be to add a new option, e.g.:
> 
>    -add-inferior [--connection NUM]
> 
> or
> 
>    -add-inferior [--inherit-connection]
> 

IMO we should support the same options as in console. Currently we don't 
allow any arguments for -add-inferior MI command though console 
add-inferior command allows several arguments. I was thinking to support 
at least "--no-connection" argument which is also supported in console. 
IMO we can add it separate commit and keep this commit for fixing the 
regression. WDYS?

> 
>> ---
>>   gdb/ChangeLog    | 9 +++++++++
>>   gdb/inferior.c   | 4 ++--
>>   gdb/inferior.h   | 5 ++++-
>>   gdb/mi/mi-main.c | 4 +++-
>>   4 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index dfee3dad157..1f342cc628f 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,12 @@
>> +2022-02-22  Umair Sair  <umair_sair@hotmail.com>
>> +
>> +	* gdb/inferior.c (switch_to_inferior_and_push_target): allowing
>> +	access to this function outside of file.
>> +	* gdb/inferior.h (switch_to_inferior_and_push_target): added extern
>> +	entry for function in header.
>> +	* gdb/mi/mi-main.c (mi_cmd_add_inferior): new inferior should use
>> +	the connection from previously selected inferior.
>> +
> 
> We no longer maintain ChangeLog in gdb, so this part is not needed.
> You're welcome to keep the ChangeLog text in the commit message in
> _addition_ to the long form description, but this is not a requirement.
> 

Yes. I actually created this patch for gdb-10-branch. Is there any 
policy to submit patches for older releases? Or should I submit patch 
for master?

>>   2021-04-23  Tom Tromey  <tromey@adacore.com>
>>   
>>   	PR gdb/27743:
>> diff --git a/gdb/inferior.c b/gdb/inferior.c
>> index 4fdddbcb55b..16e4279bc1a 100644
>> --- a/gdb/inferior.c
>> +++ b/gdb/inferior.c
>> @@ -1,6 +1,6 @@
>>   /* Multi-process control for GDB, the GNU debugger.
>>   
>> -   Copyright (C) 2008-2021 Free Software Foundation, Inc.
>> +   Copyright (C) 2008-2022 Free Software Foundation, Inc.
>>   
> 
> Not sure what's going on here.  The patch shows all the dates changing,
> but all the files you've changed already have 2022 in the upstream
> master branch.

The patch was for gdb-10-branch. I'll update patch according to the 
responses of queries above.

Thanks,
Umair Sair

> 
> Thanks,
> Andrew
> 
>>      This file is part of GDB.
>>   
>> @@ -740,7 +740,7 @@ add_inferior_with_spaces (void)
>>      NO_CONNECTION is true, push the process_stratum_target of ORG_INF
>>      to NEW_INF.  */
>>   
>> -static void
>> +void
>>   switch_to_inferior_and_push_target (inferior *new_inf,
>>   				    bool no_connection, inferior *org_inf)
>>   {
>> diff --git a/gdb/inferior.h b/gdb/inferior.h
>> index fb7d0c2bc1a..d4abd00489a 100644
>> --- a/gdb/inferior.h
>> +++ b/gdb/inferior.h
>> @@ -1,7 +1,7 @@
>>   /* Variables that describe the inferior process running under GDB:
>>      Where it is, why it stopped, and how to step it.
>>   
>> -   Copyright (C) 1986-2021 Free Software Foundation, Inc.
>> +   Copyright (C) 1986-2022 Free Software Foundation, Inc.
>>   
>>      This file is part of GDB.
>>   
>> @@ -686,4 +686,7 @@ extern struct inferior *add_inferior_with_spaces (void);
>>   /* Print the current selected inferior.  */
>>   extern void print_selected_inferior (struct ui_out *uiout);
>>   
>> +extern void switch_to_inferior_and_push_target (inferior *new_inf,
>> +            bool no_connection, inferior *org_inf);
>> +
>>   #endif /* !defined (INFERIOR_H) */
>> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
>> index e38520a98e6..65c0aa5be0b 100644
>> --- a/gdb/mi/mi-main.c
>> +++ b/gdb/mi/mi-main.c
>> @@ -1,6 +1,6 @@
>>   /* MI Command Set.
>>   
>> -   Copyright (C) 2000-2021 Free Software Foundation, Inc.
>> +   Copyright (C) 2000-2022 Free Software Foundation, Inc.
>>   
>>      Contributed by Cygnus Solutions (a Red Hat company).
>>   
>> @@ -1702,6 +1702,8 @@ mi_cmd_add_inferior (const char *command, char **argv, int argc)
>>   
>>     inf = add_inferior_with_spaces ();
>>   
>> +  switch_to_inferior_and_push_target (inf, false, current_inferior ());
>> +
>>     current_uiout->field_fmt ("inferior", "i%d", inf->num);
>>   }
>>   
>> -- 
>> 2.31.1
>