Re: [PATCH 3/5] gdb: split osabi support between gdb/ and gdbsupport/ directories

[Simon Marchi <simark@simark.ca>]

On 2024-10-08 13:11, Andrew Burgess wrote:
> In future commits I want to call set_tdesc_osabi from gdbserver/
> code.  Currently the only version of set_tdesc_osabi available to
> gdbserver takes a string representing the osabi.
> 
> The problem with this is that, having lots of calls to set_tdesc_osabi
> which all take a string is an invite for a typo to slip in.  This typo
> could potentially go unnoticed until someone tries to debug the wrong
> combination of GDB and gdbserver, at which point GDB will fail to find
> the correct gdbarch because it doesn't understand the osabi string.
> 
> It would be better if the set_tdesc_osabi calls in gdbserver could
> take an 'enum gdb_osabi' value and then convert this to the "correct"
> string internally.  In this was we are guaranteed to always have a

was -> way

> valid, known, osabi string.
> 
> This commit splits the osabi related code, which currently lives
> entirely on the GDB wide, between gdb/ and gdbsupport/.  I've moved

wide -> side

> the enum definition along with the array of osabi names into
> gdbsupport/.  Then all the functions that access the names list, and
> which convert between names and enum values are also moved.
> 
> I've taken the opportunity of this move to add a '.def' file which
> contains all the enum names along with the name strings.  This '.def'
> file is then used to create 'enum gdb_osabi' as well as the array of
> osabi name strings.  By using a '.def' file we know that the enum
> order will always match the name string array.
> 
> This commit is just a refactor, there are no user visible changes
> after this commit.  This commit doesn't change how gdbserver sets the
> target description osabi string, that will come in the next commit.
> ---
>  gdb/osabi.c                 | 91 ----------------------------------
>  gdb/osabi.h                 | 41 +---------------
>  gdbsupport/Makefile.am      |  1 +
>  gdbsupport/Makefile.in      | 15 +++---
>  gdbsupport/osabi-common.cc  | 98 +++++++++++++++++++++++++++++++++++++
>  gdbsupport/osabi-common.def | 57 +++++++++++++++++++++
>  gdbsupport/osabi-common.h   | 54 ++++++++++++++++++++

Naming nit: do we really need / want "common" in the gdbsupport file
names?  I think that the file being in gdbsupport already says it's
available to both gdb and gdbserver, so naming "common" seems
redundant (I know we have a bunch of files called "common" already).

> +const char *
> +gdbarch_osabi_name (enum gdb_osabi osabi)
> +{
> +  if (osabi >= GDB_OSABI_UNKNOWN && osabi < GDB_OSABI_INVALID)
> +    return gdb_osabi_names[osabi].pretty;
> +
> +  return gdb_osabi_names[GDB_OSABI_INVALID].pretty;
> +}
> +
> +/* See gdbsupport/osabi-commomn.h.  */

commomn -> common

Simon