Re: [PATCH 57/58] gdbserver: use unique_ptr for 'the_target'

["Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>]

On Tue, Feb 11, 2020 at 3:08 AM Tankut Baris Aktemur
<tankut.baris.aktemur@intel.com> wrote:
>
> The target op vector is about to be replaced with a process_target
> object.  In the current state, the 'set_target_ops' function takes a
> target op vector and creates a clone of it via XNEW and memcpy.  As we
> are now switching to using classes and polymorphism, this memcpy'ing
> will no longer be possible.  For proper memory managament, switch to
> using a unique_ptr.
>
> gdbserver/ChangeLog:
> 2020-02-10  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
>
>         * target.h (set_target_ops): Remove.
>         (<the_target>): Change the type to
>         std::unique_ptr<process_stratum_target>.
>         * target.c (set_target_ops): Remove.
>         (done_accessing_memory): Update the usage of the_target.
>         * fork-child.c (post_fork_inferior): Update the usage of
>         the_target.
>         * remote-utils.c (prepare_resume_reply): Ditto.
>         * linux-low.c (initialize_low): Set the value of the_target.
>         (struct linux_target_ops): Delete.
>         * lynx-low.c (initialize_low): Set the value of the_target.
>         (struct lynx_target_ops): Delete.
>         * nto-low.c (initialize_low): Set the value of the_target.
>         (struct nto_target_ops): Delete.
>         * win32-low.c (initialize_low): Set the value of the_target.
>         (struct win32_target_ops): Delete.
> ---
>  gdbserver/fork-child.c   |  2 +-
>  gdbserver/linux-low.c    |  7 ++-----
>  gdbserver/lynx-low.c     |  9 ++-------
>  gdbserver/nto-low.c      |  8 ++------
>  gdbserver/remote-utils.c |  2 +-
>  gdbserver/target.c       | 11 ++---------
>  gdbserver/target.h       |  4 +---
>  gdbserver/win32-low.c    |  7 ++-----
>  8 files changed, 13 insertions(+), 37 deletions(-)
>
> diff --git a/gdbserver/fork-child.c b/gdbserver/fork-child.c
> index 7a71ec0b1ca..3702c1a8c2e 100644
> --- a/gdbserver/fork-child.c
> +++ b/gdbserver/fork-child.c
> @@ -107,7 +107,7 @@ post_fork_inferior (int pid, const char *program)
>    atexit (restore_old_foreground_pgrp);
>  #endif
>
> -  startup_inferior (the_target, pid,
> +  startup_inferior (the_target.get (), pid,
>                     START_INFERIOR_TRAPS_EXPECTED,
>                     &cs.last_status, &cs.last_ptid);
>    current_thread->last_resume_kind = resume_stop;
> diff --git a/gdbserver/linux-low.c b/gdbserver/linux-low.c
> index cd84cef326a..87cf2fd8afb 100644
> --- a/gdbserver/linux-low.c
> +++ b/gdbserver/linux-low.c
> @@ -7512,10 +7512,6 @@ linux_get_hwcap2 (int wordsize)
>
>  static linux_process_target the_linux_target;
>
> -static process_stratum_target linux_target_ops = {
> -  &the_linux_target,
> -};
> -
>  #ifdef HAVE_LINUX_REGSETS
>  void
>  initialize_regsets_info (struct regsets_info *info)
> @@ -7533,7 +7529,8 @@ initialize_low (void)
>    struct sigaction sigchld_action;
>
>    memset (&sigchld_action, 0, sizeof (sigchld_action));
> -  set_target_ops (&linux_target_ops);
> +  the_target.reset (new process_stratum_target);
> +  the_target->pt = &the_linux_target;

Should this now be a constructor argument?

>
>    linux_ptrace_init_warnings ();
>    linux_proc_init_warnings ();
> diff --git a/gdbserver/lynx-low.c b/gdbserver/lynx-low.c
> index 778afd190b6..1377ccaf4ae 100644
> --- a/gdbserver/lynx-low.c
> +++ b/gdbserver/lynx-low.c
> @@ -739,16 +739,11 @@ lynx_process_target::sw_breakpoint_from_kind (int kind, int *size)
>
>  static lynx_process_target the_lynx_target;
>
> -/* The LynxOS target_ops vector.  */
> -
> -static process_stratum_target lynx_target_ops = {
> -  &the_lynx_target,
> -};
> -
>  void
>  initialize_low (void)
>  {
> -  set_target_ops (&lynx_target_ops);
> +  the_target.reset (new process_stratum_target);
> +  the_target->pt = &the_lynx_target;
>    the_low_target.arch_setup ();
>  }
>
> diff --git a/gdbserver/nto-low.c b/gdbserver/nto-low.c
> index cd461be8378..bbcc5046659 100644
> --- a/gdbserver/nto-low.c
> +++ b/gdbserver/nto-low.c
> @@ -946,11 +946,6 @@ nto_process_target::sw_breakpoint_from_kind (int kind, int *size)
>
>  static nto_process_target the_nto_target;
>
> -static process_stratum_target nto_target_ops = {
> -  &the_nto_target,
> -};
> -
> -
>  /* Global function called by server.c.  Initializes QNX Neutrino
>     gdbserver.  */
>
> @@ -960,7 +955,8 @@ initialize_low (void)
>    sigset_t set;
>
>    TRACE ("%s\n", __func__);
> -  set_target_ops (&nto_target_ops);
> +  the_target.reset (new process_stratum_target);
> +  the_target->pt = &the_nto_target;
>
>    /* We use SIGUSR1 to gain control after we block waiting for a process.
>       We use sigwaitevent to wait.  */
> diff --git a/gdbserver/remote-utils.c b/gdbserver/remote-utils.c
> index 316f04e32ee..14e89332db4 100644
> --- a/gdbserver/remote-utils.c
> +++ b/gdbserver/remote-utils.c
> @@ -1208,7 +1208,7 @@ prepare_resume_reply (char *buf, ptid_t ptid,
>
>         saved_thread = current_thread;
>
> -       switch_to_thread (the_target, ptid);
> +       switch_to_thread (the_target.get (), ptid);
>
>         regp = current_target_desc ()->expedite_regs;
>
> diff --git a/gdbserver/target.c b/gdbserver/target.c
> index 5c80a379f65..048fd5ad6e9 100644
> --- a/gdbserver/target.c
> +++ b/gdbserver/target.c
> @@ -27,7 +27,7 @@
>  #include <sys/types.h>
>  #include <sys/stat.h>
>
> -process_stratum_target *the_target;
> +std::unique_ptr<process_stratum_target> the_target;
>
>  int
>  set_desired_thread ()
> @@ -118,7 +118,7 @@ done_accessing_memory (void)
>
>    /* Restore the previous selected thread.  */
>    cs.general_thread = prev_general_thread;
> -  switch_to_thread (the_target, cs.general_thread);
> +  switch_to_thread (the_target.get (), cs.general_thread);
>  }
>
>  int
> @@ -267,13 +267,6 @@ target_supports_multi_process (void)
>    return the_target->pt->supports_multi_process ();
>  }
>
> -void
> -set_target_ops (process_stratum_target *target)
> -{
> -  the_target = XNEW (process_stratum_target);
> -  memcpy (the_target, target, sizeof (*the_target));
> -}
> -
>  /* Convert pid to printable format.  */
>
>  const char *
> diff --git a/gdbserver/target.h b/gdbserver/target.h
> index 1b5059c1beb..ac0ada0cd38 100644
> --- a/gdbserver/target.h
> +++ b/gdbserver/target.h
> @@ -506,9 +506,7 @@ public:
>    virtual int get_ipa_tdesc_idx ();
>  };
>
> -extern process_stratum_target *the_target;
> -
> -void set_target_ops (process_stratum_target *);
> +extern std::unique_ptr<process_stratum_target> the_target;
>
>  #define target_create_inferior(program, program_args)  \
>    the_target->pt->create_inferior (program, program_args)
> diff --git a/gdbserver/win32-low.c b/gdbserver/win32-low.c
> index 567e2ebe3db..5b4dd36f5d0 100644
> --- a/gdbserver/win32-low.c
> +++ b/gdbserver/win32-low.c
> @@ -1847,14 +1847,11 @@ win32_process_target::sw_breakpoint_from_kind (int kind, int *size)
>
>  static win32_process_target the_win32_target;
>
> -static process_stratum_target win32_target_ops = {
> -  &the_win32_target,
> -};
> -
>  /* Initialize the Win32 backend.  */
>  void
>  initialize_low (void)
>  {
> -  set_target_ops (&win32_target_ops);
> +  the_target.reset (new process_stratum_target);
> +  the_target->pt = &the_win32_target;
>    the_low_target.arch_setup ();
>  }
> --
> 2.17.1
>