Making mi -add-inferior command consistent with ci

Making mi -add-inferior command consistent with ci

[Muhammad Umair Sair]

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.
---
 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.
+
 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.
 
    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

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

[Andrew Burgess]

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.

Another option would be to add a new option, e.g.:

  -add-inferior [--connection NUM]

or

  -add-inferior [--inherit-connection]


> ---
>  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.

>  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.

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

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

[Muhammad Umair Sair]

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
> 

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

[Andrew Burgess]

Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org> writes:

> 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.

Ah, OK.  I didn't get that from the commit message.  Maybe it would be
worth expanding the commit message to make it clearer that this is a
regression.  Ideally, include an example of how GDB used to behave, and
the now broken behaviour, highlighting how things have regressed.

>
> 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?

I'd agree that ideally the same functionality would be available via the
MI as from the CLI.  As I said in my previous commit we try very hard to
maintain backward compatibility for the MI, so I'm pretty sure there are
some MI commands where the default command behaviour is not the same as
the CLI, and there are extra/different flags for the MI to allow CLI
like behaviour.  Though I can't think of any examples off the top of my
head, so maybe I'm wrong.

I 100% agree that adding new flags should be done in a separate commit
though.

Additionally, I forgot to mention in the first reviewl; we really should
have a test for this behaviour.

Thanks,
Andrew

>
>> 
>>> ---
>>>   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
>> 

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

[Muhammad Umair Sair]

Submitted a new patch.

https://sourceware.org/pipermail/gdb-patches/2022-February/186105.html

- Umair

On 2/23/22 7:37 PM, Andrew Burgess wrote:
> Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> 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.
> 
> Ah, OK.  I didn't get that from the commit message.  Maybe it would be
> worth expanding the commit message to make it clearer that this is a
> regression.  Ideally, include an example of how GDB used to behave, and
> the now broken behaviour, highlighting how things have regressed.
> 
>>
>> 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?
> 
> I'd agree that ideally the same functionality would be available via the
> MI as from the CLI.  As I said in my previous commit we try very hard to
> maintain backward compatibility for the MI, so I'm pretty sure there are
> some MI commands where the default command behaviour is not the same as
> the CLI, and there are extra/different flags for the MI to allow CLI
> like behaviour.  Though I can't think of any examples off the top of my
> head, so maybe I'm wrong.
> 
> I 100% agree that adding new flags should be done in a separate commit
> though.
> 
> Additionally, I forgot to mention in the first reviewl; we really should
> have a test for this behaviour.
> 
> Thanks,
> Andrew
> 
>>
>>>
>>>> ---
>>>>    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
>>>
>