Re: [PATCH v4 1/3] gdb: Make global feature array a per-remote target array

[Pedro Alves <pedro@palves.net>]

Hi!

On 2022-12-21 1:39 p.m., Christina Schimpe wrote:

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,20 @@
>  
>  *** Changes since GDB 13
>  
> +* Multi-target feature configuration
> +
> +  GDB now supports the individual configuration of remote target's feature

I think you meant for "remote target" to be plural, so it should be written with
the approstrophe after the plural "s", like "remote targets'".  Otherwise, if you
really meant singular, then an article seems missing after "of", as in 

  "configuration of a/the remote target's" 


> +  sets.  Based on the current selection of a target, the commands 'set remote
> +  <name>-packet (on|off|auto)' and 'show remote <name>-packet' can be used to
> +  configure a target's feature packet and to display its configuration,
> +  respectively.
> +
> +  The configuration of the packet itself applies to the currently selected
> +  target (if available).  If no target is selected, it applies to future remote
> +  connections.  Similarly, the show commands print the configuration of the
> +  currently selected target.  If no remote target is selected, the default
> +  configuration for future connections is shown.
> +
>  * MI version 1 has been removed.

...

>  
> +/* Description of a remote packet.  */
> +
> +struct packet_description
> +  {

Please fix indentation of {, and then the struct's fields accordingly (and possibly
reflow comments).  The "{" should be at column zero.  No need to repost the
patch for this.

> +    /* Name of the packet used for gdb output.  */
> +    const char *name;
> +
> +    /* Title of the packet, used by the set/show remote name-packet
> +       commands to identify the individual packages and gdb output.  */
> +    const char *title;
> +

Remove spurious empty line after last field.

> +  };
> +
> +/* Configuration of a remote packet.  */
> +
> +struct packet_config
> +  {

Ditto.

> +    /* If auto, GDB auto-detects support for this packet or feature,
> +       either through qSupported, or by trying the packet and looking
> +       at the response.  If true, GDB assumes the target supports this
> +       packet.  If false, the packet is disabled.  Configs that don't
> +       have an associated command always have this set to auto.  */
> +    enum auto_boolean detect;
> +
> +    /* Does the target support this packet?  */
> +    enum packet_support support;
> +  };
> +
This is OK with the nits above fixed.  Thank you!