Re: [PATCH 3/5] Refactor the NetBSD amd64 gdbserver support

[Simon Marchi <simark@simark.ca>]

On 2020-09-23 12:25 a.m., Kamil Rytarowski wrote:
> Replace the pre-C++ construct of netbsd_target_ops, netbsd_regset_info
> and netbsd_tdesc with C++ inheritance approach found in the Linux
> gdbserver code. Add netbsd_amd64_target, that inherits from the
> netbsd_process_target class and add proper singleton object for
> the_netbsd_target, initialized from netbsd_amd64_target.
>
> Call low_arch_setup () on post process creation, which sets machine
> specific properties of the traced process.
>
> Remove global singleton the_netbsd_target object from the generic
> gdbserver code.
>
> This refactoring introduces no functional change from the end-user
> point of view.

I just spotted some nits, but otherwise the patch is OK.

> diff --git a/gdbserver/netbsd-amd64-low.cc b/gdbserver/netbsd-amd64-low.cc
> index 9b8ea9b8aa6..2324bdbbfea 100644
> --- a/gdbserver/netbsd-amd64-low.cc
> +++ b/gdbserver/netbsd-amd64-low.cc
> @@ -155,22 +155,9 @@ netbsd_x86_64_store_gregset (struct regcache *regcache, const char *buf)
>    netbsd_x86_64_supply_gp (AMD64_GS_REGNUM, GS);
>  }
>
> -/* Implements the netbsd_target_ops.arch_setup routine.  */
> -
> -static void
> -netbsd_x86_64_arch_setup (void)
> -{
> -  struct target_desc *tdesc
> -    = amd64_create_target_description (X86_XSTATE_SSE_MASK, false, false, false);
> -
> -  init_target_desc (tdesc, amd64_expedite_regs);
> -
> -  netbsd_tdesc = tdesc;
> -}
> -
>  /* Description of all the x86-netbsd register sets.  */
>
> -struct netbsd_regset_info netbsd_target_regsets[] =
> +const static struct netbsd_regset_info netbsd_target_regsets[] =

"const static" -> "static const", just for consistency.

> @@ -504,7 +506,7 @@ netbsd_process_target::thread_alive (ptid_t ptid)
>  void
>  netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
>  {
> -  struct netbsd_regset_info *regset = netbsd_target_regsets;
> +  const netbsd_regset_info *regset = get_regs_info();

Space before parenthesis.

>    ptid_t inferior_ptid = ptid_of (current_thread);
>
>    while (regset->size >= 0)
> @@ -525,7 +527,7 @@ netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
>  void
>  netbsd_process_target::store_registers (struct regcache *regcache, int regno)
>  {
> -  struct netbsd_regset_info *regset = netbsd_target_regsets;
> +  const netbsd_regset_info *regset = get_regs_info();

Space before parenthesis.

Simon