gdb/gdbserver: move argument setting complexity into gdbsupport/

Message ID bcbc35ecc01be7109f5ab729aa25507f99b8fcd3.1766567028.git.aburgess@redhat.com
State New
Headers
Series gdb/gdbserver: move argument setting complexity into gdbsupport/ |

Commit Message

Andrew Burgess Dec. 24, 2025, 9:04 a.m. UTC
  This commit removes some uses of gdb::argv_vec from GDB.

The gdb::argv_vec class exists in part due to our need to convert from
one container type to another in a few places, and I think by using
templates we can push this conversion down into
gdbsupport/common-inferior.{cc,h} and remove the conversion logic from
higher level code in GDB.  This should make core GDB simpler.

For examples of this simplification, see python/py-inferior.c,
remote.c, and unittests/remote-arg-selftests.c, where this commit has
allowed for the removal of some code that only exists in order to
convert the container type.

Ideally I'd like to see all uses of gdb::argv_vec removed, but I'm
still not sure about the use in nat/fork-inferior.c, I think its use
there might be the best solution, so for now at least, I have no plans
to touch that code.

There should be no user visible changes after this commit.
---
 gdb/inferior.c                       |  9 ----
 gdb/inferior.h                       |  7 ++-
 gdb/python/py-inferior.c             |  7 +--
 gdb/remote.c                         | 13 +----
 gdb/unittests/remote-arg-selftests.c |  6 +--
 gdbserver/server.cc                  | 15 +++---
 gdbsupport/common-inferior.cc        | 79 +++++++++++++++++++++++++---
 gdbsupport/common-inferior.h         | 15 +++++-
 gdbsupport/remote-args.cc            |  8 ---
 gdbsupport/remote-args.h             | 22 +++++++-
 10 files changed, 126 insertions(+), 55 deletions(-)


base-commit: 5d8562e89a612f2d0e5c9e0813a0d37620eff78c
  

Patch

diff --git a/gdb/inferior.c b/gdb/inferior.c
index 8131f23b938..0246203bec9 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -165,15 +165,6 @@  inferior::tty ()
   return m_terminal;
 }
 
-/* See inferior.h.  */
-
-void
-inferior::set_args (gdb::array_view<char * const> args,
-		    bool escape_shell_char)
-{
-  set_args (construct_inferior_arguments (args, escape_shell_char));
-}
-
 void
 inferior::set_arch (gdbarch *arch)
 {
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 5b499a207b4..124120e028d 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -62,6 +62,7 @@  struct thread_info;
 #include "displaced-stepping.h"
 
 #include "gdbsupport/unordered_map.h"
+#include "gdbsupport/common-inferior.h"
 
 struct infcall_suspend_state;
 struct infcall_control_state;
@@ -545,7 +546,11 @@  class inferior : public refcounted_object,
      ESCAPE_SHELL_CHAR is true all special shell characters in ARGS are
      escaped, When false only the characters that GDB sees as special will
      be escaped.  See construct_inferior_arguments for more details.  */
-  void set_args (gdb::array_view<char * const> args, bool escape_shell_char);
+  template<typename T>
+  void set_args (gdb::array_view<T const> args, bool escape_shell_char)
+  {
+    this->set_args (construct_inferior_arguments (args, escape_shell_char));
+  }
 
   /* Get the argument string to use when running this inferior.
 
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 0351a646a20..57c4a948450 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -924,11 +924,8 @@  infpy_set_args (PyObject *self, PyObject *value, void *closure)
 	    return -1;
 	  args.push_back (std::move (str));
 	}
-      std::vector<char *> argvec;
-      for (const auto &arg : args)
-	argvec.push_back (arg.get ());
-      gdb::array_view<char * const> view (argvec.data (), argvec.size ());
-      inf->inferior->set_args (view, true);
+      gdb::array_view<gdb::unique_xmalloc_ptr<char> const> args_view (args);
+      inf->inferior->set_args (args_view, true);
     }
   else
     {
diff --git a/gdb/remote.c b/gdb/remote.c
index dbe3bb197b3..18ac47ee8a9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -83,7 +83,6 @@ 
 #include "gdbsupport/selftest.h"
 #include "cli/cli-style.h"
 #include "gdbsupport/remote-args.h"
-#include "gdbsupport/gdb_argv_vec.h"
 
 /* The remote target.  */
 
@@ -12764,21 +12763,13 @@  test_remote_args_command (const char *args, int from_tty)
   for (const std::string &a : split_args)
     gdb_printf ("  (%s)\n", a.c_str ());
 
-  gdb::argv_vec tmp_split_args;
-  for (const std::string &a : split_args)
-    tmp_split_args.emplace_back (xstrdup (a.c_str ()));
-
-  std::string joined_args = gdb::remote_args::join (tmp_split_args.get ());
+  std::string joined_args = gdb::remote_args::join (split_args);
 
   gdb_printf ("Output (%s)\n", joined_args.c_str ());
 
   std::vector<std::string> resplit = gdb::remote_args::split (joined_args);
 
-  tmp_split_args.clear ();
-  for (const std::string &a : resplit)
-    tmp_split_args.emplace_back (xstrdup (a.c_str ()));
-
-  std::string rejoined = gdb::remote_args::join (tmp_split_args.get ());
+  std::string rejoined = gdb::remote_args::join (resplit);
 
   if (joined_args != rejoined || split_args != resplit)
     {
diff --git a/gdb/unittests/remote-arg-selftests.c b/gdb/unittests/remote-arg-selftests.c
index 17b8d1d0b95..2d9235e5611 100644
--- a/gdb/unittests/remote-arg-selftests.c
+++ b/gdb/unittests/remote-arg-selftests.c
@@ -22,7 +22,6 @@ 
 #include "gdbsupport/buildargv.h"
 #include "gdbsupport/common-inferior.h"
 #include "gdbsupport/remote-args.h"
-#include "gdbsupport/gdb_argv_vec.h"
 
 namespace selftests {
 namespace remote_args_tests {
@@ -107,11 +106,8 @@  self_test ()
 	}
 
       /* Now join the arguments.  */
-      gdb::argv_vec split_args_c_str;
-      for (const std::string &s : split_args)
-	split_args_c_str.push_back (xstrdup (s.c_str ()));
       std::string joined_args
-	= gdb::remote_args::join (split_args_c_str.get ());
+	= gdb::remote_args::join (split_args);
 
       if (run_verbose ())
 	debug_printf ("Joined (%s), expected (%s)\n",
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 8d2fef40d0a..7c5a37853eb 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -49,7 +49,6 @@ 
 #include "gdbsupport/gdb_select.h"
 #include "gdbsupport/scoped_restore.h"
 #include "gdbsupport/search.h"
-#include "gdbsupport/gdb_argv_vec.h"
 #include "gdbsupport/remote-args.h"
 
 #include <getopt.h>
@@ -3442,7 +3441,7 @@  handle_v_run (char *own_buf)
 {
   client_state &cs = get_client_state ();
   char *p, *next_p;
-  gdb::argv_vec new_argv;
+  std::vector<gdb::unique_xmalloc_ptr<char>> new_argv;
   gdb::unique_xmalloc_ptr<char> new_program_name;
   int i;
 
@@ -3462,7 +3461,7 @@  handle_v_run (char *own_buf)
       else if (p == next_p)
 	{
 	  /* Empty argument.  */
-	  new_argv.push_back (xstrdup (""));
+	  new_argv.push_back (make_unique_xstrdup (""));
 	}
       else
 	{
@@ -3479,7 +3478,7 @@  handle_v_run (char *own_buf)
 	  if (i == 0)
 	    new_program_name = std::move (arg);
 	  else
-	    new_argv.push_back (arg.release ());
+	    new_argv.push_back (std::move (arg));
 	}
       if (*next_p == '\0')
 	break;
@@ -3500,18 +3499,18 @@  handle_v_run (char *own_buf)
 
   if (cs.single_inferior_argument)
     {
-      if (new_argv.get ().size () > 1)
+      if (new_argv.size () > 1)
 	{
 	  write_enn (own_buf);
 	  return;
 	}
-      else if (new_argv.get ().size () == 1)
-	program_args = std::string (new_argv.get ()[0]);
+      else if (new_argv.size () == 1)
+	program_args = std::string (new_argv[0].get ());
       else
 	program_args.clear ();
     }
   else
-    program_args = gdb::remote_args::join (new_argv.get ());
+    program_args = gdb::remote_args::join (new_argv);
 
   try
     {
diff --git a/gdbsupport/common-inferior.cc b/gdbsupport/common-inferior.cc
index a07b556014a..da67399367c 100644
--- a/gdbsupport/common-inferior.cc
+++ b/gdbsupport/common-inferior.cc
@@ -130,11 +130,50 @@  escape_gdb_characters (const char * arg)
   return escape_characters (arg, special);
 }
 
-/* See common-inferior.h.  */
+/* Template function that converts a T to a 'const char *'.  */
 
-std::string
-construct_inferior_arguments (gdb::array_view<char * const> argv,
-			      bool escape_shell_char)
+template<typename T> const char * to_char_ptr (T &arg);
+
+/* A no-op implementation that takes a 'const char *'.  */
+
+template<>
+const char *
+to_char_ptr (char * const &arg)
+{
+  return arg;
+}
+
+/* An implementation that gets a 'const char *' from a
+   gdb::unique_xmalloc_ptr<char> object.  */
+
+template<>
+const char *
+to_char_ptr (const gdb::unique_xmalloc_ptr<char> &arg)
+{
+  return arg.get ();
+}
+
+/* An implementation that gets a 'const char *' from a std::string.  */
+
+template<>
+const char *
+to_char_ptr (const std::string &arg)
+{
+  return arg.c_str ();
+}
+
+/* Helper function for the two construct_inferior_arguments overloads
+   below.  Accept a gdb::array_view over objects of type T.  Combine each T
+   into a std::string by calling an appropriate escape function, with a
+   white space character added between each T.  When ESCAPE_SHELL_CHAR is
+   true then any special shell characters in elemets of ARGV will be
+   escaped.  When ESCAPE_SHELL_CHAR is false only the characters that GDB
+   sees as special (quotes and whitespace) are escaped.  */
+
+template<typename T>
+static std::string
+construct_inferior_arguments_1
+  (gdb::array_view<T const> argv, bool escape_shell_char)
 {
   /* Select the desired escape function.  */
   const auto escape_func = (escape_shell_char
@@ -143,13 +182,41 @@  construct_inferior_arguments (gdb::array_view<char * const> argv,
 
   std::string result;
 
-  for (const char *a : argv)
+  for (const T &a : argv)
     {
       if (!result.empty ())
 	result += " ";
 
-      result += escape_func (a);
+      result += escape_func (to_char_ptr (a));
     }
 
   return result;
 }
+
+/* See common-inferior.h.  */
+
+std::string
+construct_inferior_arguments
+  (gdb::array_view<gdb::unique_xmalloc_ptr<char> const> argv,
+   bool escape_shell_char)
+{
+  return construct_inferior_arguments_1 (argv, escape_shell_char);
+}
+
+/* See common-inferior.h.  */
+
+std::string
+construct_inferior_arguments (gdb::array_view<char * const> argv,
+			      bool escape_shell_char)
+{
+  return construct_inferior_arguments_1 (argv, escape_shell_char);
+}
+
+/* See common-inferior.h.  */
+
+std::string
+construct_inferior_arguments (gdb::array_view<std::string const> argv,
+			      bool escape_shell_char)
+{
+  return construct_inferior_arguments_1 (argv, escape_shell_char);
+}
diff --git a/gdbsupport/common-inferior.h b/gdbsupport/common-inferior.h
index 200c629ec91..06a587d0c06 100644
--- a/gdbsupport/common-inferior.h
+++ b/gdbsupport/common-inferior.h
@@ -58,7 +58,20 @@  extern bool startup_with_shell;
    escaped.  When ESCAPE_SHELL_CHAR is false only the characters that GDB
    sees as special (quotes and whitespace) are escaped.  */
 extern std::string
-construct_inferior_arguments (gdb::array_view<char * const> argv,
+construct_inferior_arguments
+  (gdb::array_view<gdb::unique_xmalloc_ptr<char> const> args,
+   bool escape_shell_char);
+
+/* An overload of the above that takes an array of raw pointers.  */
+
+extern std::string
+construct_inferior_arguments (gdb::array_view<char * const> args,
+			      bool escape_shell_char);
+
+/* An overload of the above that takes an array of std::string.  */
+
+extern std::string
+construct_inferior_arguments (gdb::array_view<std::string const> args,
 			      bool escape_shell_char);
 
 #endif /* GDBSUPPORT_COMMON_INFERIOR_H */
diff --git a/gdbsupport/remote-args.cc b/gdbsupport/remote-args.cc
index 2493433cc62..e6f5d699af6 100644
--- a/gdbsupport/remote-args.cc
+++ b/gdbsupport/remote-args.cc
@@ -33,11 +33,3 @@  gdb::remote_args::split (const std::string &args)
 
   return results;
 }
-
-/* See remote-args.h.  */
-
-std::string
-gdb::remote_args::join (const std::vector<char *> &args)
-{
-  return construct_inferior_arguments (args, true);
-}
diff --git a/gdbsupport/remote-args.h b/gdbsupport/remote-args.h
index 0533da6e679..f4ffd499f2d 100644
--- a/gdbsupport/remote-args.h
+++ b/gdbsupport/remote-args.h
@@ -20,6 +20,8 @@ 
 #ifndef GDBSUPPORT_REMOTE_ARGS_H
 #define GDBSUPPORT_REMOTE_ARGS_H
 
+#include "gdbsupport/common-inferior.h"
+
 /* The functions declared here are used when passing inferior arguments
    from GDB to gdbserver.
 
@@ -51,7 +53,25 @@  extern std::vector<std::string> split (const std::string &args);
    passed through ::join we will get back the string 'a\ b' (without the
    single quotes), that is, we choose to escape the white space, rather
    than wrap the argument in quotes.  */
-extern std::string join (const std::vector<char *> &args);
+inline std::string
+join (const std::vector<gdb::unique_xmalloc_ptr<char>> &args)
+{
+  return construct_inferior_arguments (args, true);
+}
+
+/* As above, but with alternative argument type.  */
+inline std::string
+join (const std::vector<char *> &args)
+{
+  return construct_inferior_arguments (args, true);
+}
+
+/* As above, but with alternative argument type.  */
+inline std::string
+join (const std::vector<std::string> &args)
+{
+  return construct_inferior_arguments (args, true);
+}
 
 } /* namespace remote_args */