[PATCH 0/4] Cleanups, remove get_exec_file

[PATCH 0/4] Cleanups, remove get_exec_file

[Simon Marchi]

Some cleanups and simplifications around getting the exec filename.  No
behavior change intended.

Simon Marchi (4):
  gdb: make progspace::exec_filename private, add getter / setter
  gdb: replace `get_exec_file (0)` calls with
    `current_program_space->exec_filename ()`
  gdb: remove dead code in nto-procfs.c
  gdb: remove get_exec_file

 gdb/bsd-kvm.c                |  3 +--
 gdb/corefile.c               | 15 ---------------
 gdb/darwin-nat.c             |  3 +++
 gdb/elf-none-tdep.c          |  4 ++--
 gdb/exec.c                   | 27 ++++++++++++++++++---------
 gdb/exec.h                   |  5 +++++
 gdb/fbsd-tdep.c              |  4 ++--
 gdb/gnu-nat.c                |  3 +++
 gdb/go32-nat.c               |  6 ++----
 gdb/inf-ptrace.c             |  3 +++
 gdb/infcmd.c                 |  5 ++---
 gdb/inferior.c               |  8 ++++----
 gdb/mi/mi-main.c             |  7 ++-----
 gdb/nat/fork-inferior.c      | 16 +++++-----------
 gdb/nto-procfs.c             | 15 +++++----------
 gdb/procfs.c                 | 10 +++++++---
 gdb/progspace.c              | 14 +++++++-------
 gdb/progspace.h              | 16 ++++++++++++----
 gdb/python/py-progspace.c    |  2 +-
 gdb/remote.c                 |  2 +-
 gdb/symfile.c                |  4 +++-
 gdb/target.c                 |  4 ++--
 gdbserver/server.cc          | 11 -----------
 gdbsupport/common-inferior.h |  5 -----
 24 files changed, 90 insertions(+), 102 deletions(-)


base-commit: cc08f5580eefc467a9a5fa8f66eeaf6a1a9bbac6
-- 
2.45.1

[PATCH 1/4] gdb: make progspace::exec_filename private, add getter / setter

[Simon Marchi]

Just like the title says... I think this makes things a bit clearer, for
instance where the exec filename is set.  It also makes the read call
sites a bit nicer, avoiding the `.get ()`.

Change-Id: If8b58ae8f6270c8a34b868f6ca06128c6671ea3c
---
 gdb/corefile.c            |  5 +++--
 gdb/exec.c                | 12 ++++++------
 gdb/inferior.c            |  8 ++++----
 gdb/mi/mi-main.c          |  7 ++-----
 gdb/progspace.c           | 14 +++++++-------
 gdb/progspace.h           | 16 ++++++++++++----
 gdb/python/py-progspace.c |  2 +-
 7 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/gdb/corefile.c b/gdb/corefile.c
index 044c084ab6f3..3ae3fc7b6337 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -81,8 +81,9 @@ validate_files (void)
 const char *
 get_exec_file (int err)
 {
-  if (current_program_space->exec_filename != nullptr)
-    return current_program_space->exec_filename.get ();
+  if (current_program_space->exec_filename () != nullptr)
+    return current_program_space->exec_filename ();
+
   if (!err)
     return NULL;
 
diff --git a/gdb/exec.c b/gdb/exec.c
index 88915260d609..4fe4d3bb50ad 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -456,15 +456,15 @@ exec_file_attach (const char *filename, int from_tty)
 
       /* gdb_realpath_keepfile resolves symlinks on the local
 	 filesystem and so cannot be used for "target:" files.  */
-      gdb_assert (current_program_space->exec_filename == nullptr);
+      gdb_assert (current_program_space->exec_filename () == nullptr);
       if (load_via_target)
-	current_program_space->exec_filename
-	  = (make_unique_xstrdup
+	current_program_space->set_exec_filename
+	  (make_unique_xstrdup
 	     (bfd_get_filename (current_program_space->exec_bfd ())));
       else
-	current_program_space->exec_filename
-	  = make_unique_xstrdup (gdb_realpath_keepfile
-				   (scratch_pathname).c_str ());
+	current_program_space->set_exec_filename
+	  (make_unique_xstrdup (gdb_realpath_keepfile
+				  (scratch_pathname).c_str ()));
 
       if (!bfd_check_format_matches (current_program_space->exec_bfd (),
 				     bfd_object, &matching))
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 0522cb5c14d5..6a197679902b 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -519,7 +519,7 @@ void
 print_selected_inferior (struct ui_out *uiout)
 {
   struct inferior *inf = current_inferior ();
-  const char *filename = inf->pspace->exec_filename.get ();
+  const char *filename = inf->pspace->exec_filename ();
 
   if (filename == NULL)
     filename = _("<noexec>");
@@ -613,8 +613,8 @@ print_inferior (struct ui_out *uiout, const char *requested_inferiors)
       std::string conn = uiout_field_connection (inf->process_target ());
       uiout->field_string ("connection-id", conn);
 
-      if (inf->pspace->exec_filename != nullptr)
-	uiout->field_string ("exec", inf->pspace->exec_filename.get (),
+      if (inf->pspace->exec_filename () != nullptr)
+	uiout->field_string ("exec", inf->pspace->exec_filename (),
 			     file_name_style.style ());
       else
 	uiout->field_skip ("exec");
@@ -750,7 +750,7 @@ inferior_command (const char *args, int from_tty)
     {
       inf = current_inferior ();
       gdb_assert (inf != nullptr);
-      const char *filename = inf->pspace->exec_filename.get ();
+      const char *filename = inf->pspace->exec_filename ();
 
       if (filename == nullptr)
 	filename = _("<noexec>");
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index a758b689ae72..5bcb5f7ee8ce 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -651,11 +651,8 @@ print_one_inferior (struct inferior *inferior, bool recurse,
       if (inferior->pid != 0)
 	uiout->field_signed ("pid", inferior->pid);
 
-      if (inferior->pspace->exec_filename != nullptr)
-	{
-	  uiout->field_string ("executable",
-			       inferior->pspace->exec_filename.get ());
-	}
+      if (inferior->pspace->exec_filename () != nullptr)
+	uiout->field_string ("executable", inferior->pspace->exec_filename ());
 
       if (inferior->pid != 0)
 	{
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 5be514d45b0d..d5b5ef20023b 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -209,7 +209,7 @@ program_space::exec_close ()
 
       remove_target_sections (saved_ebfd);
 
-      exec_filename.reset (nullptr);
+      m_exec_filename.reset ();
     }
 }
 
@@ -223,8 +223,8 @@ clone_program_space (struct program_space *dest, struct program_space *src)
 
   set_current_program_space (dest);
 
-  if (src->exec_filename != NULL)
-    exec_file_attach (src->exec_filename.get (), 0);
+  if (src->exec_filename () != nullptr)
+    exec_file_attach (src->exec_filename (), 0);
 
   if (src->symfile_object_file != NULL)
     symbol_file_add_main (objfile_name (src->symfile_object_file),
@@ -277,8 +277,8 @@ print_program_space (struct ui_out *uiout, int requested)
       if (requested != -1 && pspace->num != requested)
 	continue;
 
-      if (pspace->exec_filename != nullptr)
-	longest_exec_name = std::max (strlen (pspace->exec_filename.get ()),
+      if (pspace->exec_filename () != nullptr)
+	longest_exec_name = std::max (strlen (pspace->exec_filename ()),
 				      longest_exec_name);
 
       ++count;
@@ -310,8 +310,8 @@ print_program_space (struct ui_out *uiout, int requested)
 
       uiout->field_signed ("id", pspace->num);
 
-      if (pspace->exec_filename != nullptr)
-	uiout->field_string ("exec", pspace->exec_filename.get (),
+      if (pspace->exec_filename () != nullptr)
+	uiout->field_string ("exec", pspace->exec_filename (),
 			     file_name_style.style ());
       else
 	uiout->field_skip ("exec");
diff --git a/gdb/progspace.h b/gdb/progspace.h
index bc24ef93733c..82c0a743f326 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -292,6 +292,15 @@ struct program_space
   intrusive_list<solib> &solibs ()
   { return so_list; }
 
+  /* Similar to `bfd_get_filename (exec_bfd ())` but in original form given
+     by user, without symbolic links and pathname resolved.  It is not nullptr
+     iff `exec_bfd ()` is not nullptr.  */
+  const char *exec_filename () const
+  { return m_exec_filename.get (); }
+
+  void set_exec_filename (gdb::unique_xmalloc_ptr<char> filename)
+  { m_exec_filename = std::move (filename); }
+
   /* Close and clear exec_bfd.  If we end up with no target sections
      to read memory from, this unpushes the exec_ops target.  */
   void exec_close ();
@@ -352,10 +361,6 @@ struct program_space
   gdb_bfd_ref_ptr ebfd;
   /* The last-modified time, from when the exec was brought in.  */
   long ebfd_mtime = 0;
-  /* Similar to bfd_get_filename (exec_bfd) but in original form given
-     by user, without symbolic links and pathname resolved.  It is not
-     NULL iff EBFD is not NULL.  */
-  gdb::unique_xmalloc_ptr<char> exec_filename;
 
   /* Binary file diddling handle for the core file.  */
   gdb_bfd_ref_ptr cbfd;
@@ -414,6 +419,9 @@ struct program_space
   /* The set of target sections matching the sections mapped into
      this program space.  Managed by both exec_ops and solib.c.  */
   std::vector<target_section> m_target_sections;
+
+  /* See `exec_filename`.  */
+  gdb::unique_xmalloc_ptr<char> m_exec_filename;
 };
 
 /* The list of all program spaces.  There's always at least one.  */
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index a5b22ce9d5c8..5bc0015d7288 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -148,7 +148,7 @@ pspy_get_exec_file (PyObject *self, void *closure)
 
   PSPY_REQUIRE_VALID (obj);
 
-  const char *filename = obj->pspace->exec_filename.get ();
+  const char *filename = obj->pspace->exec_filename ();
   if (filename != nullptr)
     return host_string_to_python_string (filename).release ();
 
-- 
2.45.1

[PATCH 2/4] gdb: replace `get_exec_file (0)` calls with `current_program_space->exec_filename ()`

[Simon Marchi]

Calls of `get_exec_file (0)` are equivalent to just getting the exec
filename from the current program space.  I'm looking to remove
`get_exec_file`, so replace these uses with
`current_program_space->exec_filename ()`.

Remove the `err` parameter of `get_exec_wrapper` since all the calls
that remain pass 1, meaning to error out if no executable is specified.

Change-Id: I7729ea4c7f03dbb046211cc5aa3858ab3a551965
---
 gdb/bsd-kvm.c                | 3 +--
 gdb/corefile.c               | 5 +----
 gdb/elf-none-tdep.c          | 4 ++--
 gdb/exec.c                   | 6 +++---
 gdb/fbsd-tdep.c              | 4 ++--
 gdb/go32-nat.c               | 2 +-
 gdb/infcmd.c                 | 5 ++---
 gdb/nat/fork-inferior.c      | 2 +-
 gdb/nto-procfs.c             | 2 +-
 gdb/procfs.c                 | 7 ++++---
 gdb/remote.c                 | 2 +-
 gdb/symfile.c                | 2 +-
 gdb/target.c                 | 4 ++--
 gdbserver/server.cc          | 4 ++--
 gdbsupport/common-inferior.h | 6 +++---
 15 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
index a1020b05b193..7ef8c349a6e7 100644
--- a/gdb/bsd-kvm.c
+++ b/gdb/bsd-kvm.c
@@ -108,7 +108,6 @@ static void
 bsd_kvm_target_open (const char *arg, int from_tty)
 {
   char errbuf[_POSIX2_LINE_MAX];
-  const char *execfile = NULL;
   kvm_t *temp_kd;
   std::string filename;
 
@@ -121,7 +120,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
 	filename = gdb_abspath (filename.c_str ());
     }
 
-  execfile = get_exec_file (0);
+  const char *execfile = current_program_space->exec_filename ();
   temp_kd = kvm_openfiles (execfile, filename.c_str (), NULL,
 			   write_files ? O_RDWR : O_RDONLY, errbuf);
   if (temp_kd == NULL)
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 3ae3fc7b6337..aa35e3aa4e1f 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -79,14 +79,11 @@ validate_files (void)
 /* See gdbsupport/common-inferior.h.  */
 
 const char *
-get_exec_file (int err)
+get_exec_file ()
 {
   if (current_program_space->exec_filename () != nullptr)
     return current_program_space->exec_filename ();
 
-  if (!err)
-    return NULL;
-
   error (_("No executable file specified.\n\
 Use the \"file\" or \"exec-file\" command."));
 }
diff --git a/gdb/elf-none-tdep.c b/gdb/elf-none-tdep.c
index 00d46992b248..8763aa85406f 100644
--- a/gdb/elf-none-tdep.c
+++ b/gdb/elf-none-tdep.c
@@ -42,9 +42,9 @@ elf_none_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
   std::string psargs;
   static const size_t fname_len = 16;
   static const size_t psargs_len = 80;
-  if (get_exec_file (0))
+  if (current_program_space->exec_filename () != nullptr)
     {
-      const char *exe = get_exec_file (0);
+      const char *exe = current_program_space->exec_filename ();
       fname = lbasename (exe);
       psargs = std::string (exe);
 
diff --git a/gdb/exec.c b/gdb/exec.c
index 4fe4d3bb50ad..496f3b130708 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -215,7 +215,7 @@ validate_exec_file (int from_tty)
   if (exec_file_mismatch_mode == exec_file_mismatch_off)
     return;
 
-  const char *current_exec_file = get_exec_file (0);
+  const char *current_exec_file = current_program_space->exec_filename ();
   struct inferior *inf = current_inferior ();
   /* Try to determine a filename from the process itself.  */
   const char *pid_exec_file = target_pid_to_exec_file (inf->pid);
@@ -233,7 +233,7 @@ validate_exec_file (int from_tty)
      did not change).  If exec file changed, reopen_exec_file has
      allocated another file name, so get_exec_file again.  */
   reopen_exec_file ();
-  current_exec_file = get_exec_file (0);
+  current_exec_file = current_program_space->exec_filename ();
 
   const bfd_build_id *exec_file_build_id
     = build_id_bfd_get (current_program_space->exec_bfd ());
@@ -314,7 +314,7 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
   symfile_add_flags add_flags = 0;
 
   /* Do nothing if we already have an executable filename.  */
-  if (get_exec_file (0) != NULL)
+  if (current_program_space->exec_filename () != nullptr)
     return;
 
   /* Try to determine a filename from the process itself.  */
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index a80f604fa780..000635748de5 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -684,9 +684,9 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
 
   gdb_assert (gdbarch_iterate_over_regset_sections_p (gdbarch));
 
-  if (get_exec_file (0))
+  if (current_program_space->exec_filename () != nullptr)
     {
-      const char *fname = lbasename (get_exec_file (0));
+      const char *fname = lbasename (current_program_space->exec_filename ());
       std::string psargs = fname;
 
       const std::string &infargs = current_inferior ()->args ();
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index 386c73ce26b1..f59a7ed5a2bc 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -684,7 +684,7 @@ go32_nat_target::create_inferior (const char *exec_file,
   /* If no exec file handed to us, get it from the exec-file command -- with
      a good, common error message if none is specified.  */
   if (exec_file == 0)
-    exec_file = get_exec_file (1);
+    exec_file = get_exec_file ();
 
   resume_signal = -1;
   resume_is_step = 0;
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 02f6f0b3683f..c40500b62d9f 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -367,7 +367,6 @@ enum run_how
 static void
 run_command_1 (const char *args, int from_tty, enum run_how run_how)
 {
-  const char *exec_file;
   struct ui_out *uiout = current_uiout;
   struct target_ops *run_target;
   int async_exec;
@@ -421,7 +420,7 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
       tbreak_command (arg.c_str (), 0);
     }
 
-  exec_file = get_exec_file (0);
+  const char *exec_file = current_program_space->exec_filename ();
 
   /* We keep symbols from add-symbol-file, on the grounds that the
      user might want to add some symbols before running the program
@@ -2497,7 +2496,7 @@ setup_inferior (int from_tty)
 
   /* If no exec file is yet known, try to determine it from the
      process itself.  */
-  if (get_exec_file (0) == nullptr)
+  if (current_program_space->exec_filename () == nullptr)
     exec_file_locate_attach (inferior_ptid.pid (), 1, from_tty);
   else
     {
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index c1082eb04411..172eef0f93b0 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -282,7 +282,7 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
   /* If no exec file handed to us, get it from the exec-file command
      -- with a good, common error message if none is specified.  */
   if (exec_file_arg == NULL)
-    exec_file = get_exec_file (1);
+    exec_file = get_exec_file ();
   else
     exec_file = exec_file_arg;
 
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 95a75b46d4fc..c3a45796dcc7 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -1190,7 +1190,7 @@ nto_procfs_target::create_inferior (const char *exec_file,
 
   argv = xmalloc ((allargs.size () / (unsigned) 2 + 2) *
 		  sizeof (*argv));
-  argv[0] = const_cast<char *> (get_exec_file (1));
+  argv[0] = const_cast<char *> (get_exec_file ());
   if (!argv[0])
     {
       if (exec_file)
diff --git a/gdb/procfs.c b/gdb/procfs.c
index b5bda4dfb858..93d1113893d2 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -3577,11 +3577,12 @@ procfs_target::make_corefile_notes (bfd *obfd, int *note_size)
   gdb::unique_xmalloc_ptr<char> note_data;
   enum gdb_signal stop_signal;
 
-  if (get_exec_file (0))
+  if (const auto exec_filename = current_program_space->exec_filename ();
+      exec_filename != nullptr)
     {
-      strncpy (fname, lbasename (get_exec_file (0)), sizeof (fname));
+      strncpy (fname, lbasename (exec_filename), sizeof (fname));
       fname[sizeof (fname) - 1] = 0;
-      strncpy (psargs, get_exec_file (0), sizeof (psargs));
+      strncpy (psargs, exec_filename, sizeof (psargs));
       psargs[sizeof (psargs) - 1] = 0;
 
       const std::string &inf_args = current_inferior ()->args ();
diff --git a/gdb/remote.c b/gdb/remote.c
index 965427c96a0d..38e406078982 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2870,7 +2870,7 @@ remote_target::remote_add_inferior (bool fake_pid_p, int pid, int attached,
 
   /* If no main executable is currently open then attempt to
      open the file that was executed to create this inferior.  */
-  if (try_open_exec && get_exec_file (0) == NULL)
+  if (try_open_exec && current_program_space->exec_filename () == nullptr)
     exec_file_locate_attach (pid, 0, 1);
 
   /* Check for exec file mismatch, and let the user solve it.  */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 7ebb07d5394b..78e68139c6f7 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1843,7 +1843,7 @@ load_command (const char *arg, int from_tty)
     {
       const char *parg, *prev;
 
-      arg = get_exec_file (1);
+      arg = get_exec_file ();
 
       /* We may need to quote this string so buildargv can pull it
 	 apart.  */
diff --git a/gdb/target.c b/gdb/target.c
index ddf10c28e1c7..de360f044562 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3572,7 +3572,7 @@ target_announce_detach (int from_tty)
     return;
 
   pid = inferior_ptid.pid ();
-  exec_file = get_exec_file (0);
+  exec_file = current_program_space->exec_filename ();
   if (exec_file == nullptr)
     gdb_printf ("Detaching from pid %s\n",
 		target_pid_to_str (ptid_t (pid)).c_str ());
@@ -3589,7 +3589,7 @@ target_announce_attach (int from_tty, int pid)
   if (!from_tty)
     return;
 
-  const char *exec_file = get_exec_file (0);
+  const char *exec_file = current_program_space->exec_filename ();
 
   if (exec_file != nullptr)
     gdb_printf ("Attaching to program: %s, %s\n", exec_file,
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 789af36d9a42..82f4318457cb 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -285,9 +285,9 @@ get_exec_wrapper ()
 /* See gdbsupport/common-inferior.h.  */
 
 const char *
-get_exec_file (int err)
+get_exec_file ()
 {
-  if (err && program_path.get () == NULL)
+  if (program_path.get () == NULL)
     error (_("No executable file specified."));
 
   return program_path.get ();
diff --git a/gdbsupport/common-inferior.h b/gdbsupport/common-inferior.h
index bc6afd65f2b5..9c19e9d9fdd9 100644
--- a/gdbsupport/common-inferior.h
+++ b/gdbsupport/common-inferior.h
@@ -28,9 +28,9 @@
 extern const char *get_exec_wrapper ();
 
 /* Return the name of the executable file as a string.
-   ERR nonzero means get error if there is none specified;
-   otherwise return 0 in that case.  */
-extern const char *get_exec_file (int err);
+
+   Error out if no executable is specified.  */
+extern const char *get_exec_file ();
 
 /* Return the inferior's current working directory.
 
-- 
2.45.1

[PATCH 3/4] gdb: remove dead code in nto-procfs.c

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

`get_exec_file()` never returns nullptr, so remove some dead code that
check for a nullptr return.

Change-Id: I9eff2a013d602588aaf4477a22cf45f2bc417c6a
---
 gdb/nto-procfs.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index c3a45796dcc7..9ec214771271 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -1191,14 +1191,6 @@ nto_procfs_target::create_inferior (const char *exec_file,
   argv = xmalloc ((allargs.size () / (unsigned) 2 + 2) *
 		  sizeof (*argv));
   argv[0] = const_cast<char *> (get_exec_file ());
-  if (!argv[0])
-    {
-      if (exec_file)
-	argv[0] = exec_file;
-      else
-	return;
-    }
-
   args = xstrdup (allargs.c_str ());
   breakup_args (args, (exec_file != NULL) ? &argv[1] : &argv[0]);
 
-- 
2.45.1

[PATCH 4/4] gdb: remove get_exec_file

[Simon Marchi]

I believe that the get_exec_file function is unnecessary, and the code
can be simplified if we remove it.

Consider for instance when you "run" a program on Linux with native
debugging.

 1. run_command_1 obtains the executable file from
    `current_program_space->exec_filename ()`
 2. it passes it to `run_target->create_inferior()`, which is
    `inf_ptrace_target::create_inferior()` in this case, which then
    passes it to `fork_inferior()`
 3. `fork_inferior()` then has a fallback, where if the passed exec file
    is nullptr, it gets its from `get_exec_file()`.
 4. `get_exec_file()` returns `current_program_space->exec_filename ()`
    - just like the things we started with - or errors out if the
    current program space doesn't have a specified executable.

If there's no exec filename passed in step 1, there's not going to be
any in step 4, so it seems pointless to call `get_exec_file()`, we could
just error out when `exec_file` is nullptr.  But we can't error out
directly in `fork_inferior()`, since the error is GDB-specific, and that
function is shared with GDBserver.

Speaking of GDBserver, all code paths that lead to `fork_inferior()`
provide a non-nullptr exec file.

Therefore, to simplify things:

 - Make `fork_inferior()` assume that the passed exec file is not
   nullptr, don't call `get_exec_file()`
 - Change some targets (darwin-nat, go32-nat, gnu-nat, inf-ptrace,
   nto-procfs, procfs) to error out when the exec file passed to their
   create_inferior method is nullptr.  Some targets are fine with a
   nullptr exec file, so we can't check that in `run_command_1()`.
 - Add the `no_executable_specified_error()` function, which re-uses the
   error message that `get_exec_file()` had.
 - Change some targets (go32-nat, nto-procfs) to not call
   `get_exec_file()`, since it's pointless for the same reason as in the
   example above, if it returns, it's going the be the same value as the
   `exec_file` parameter.  Just rely on `exec_file`.
 - Remove the final use of `get_exec_file()`, in `load_command()`.
 - Remove the `get_exec_file()` implementations in GDB and GDBserver and
   remove the shared declaration.

Change-Id: I601c16498e455f7baa1f111a179da2f6c913baa3
---
 gdb/corefile.c               | 13 -------------
 gdb/darwin-nat.c             |  3 +++
 gdb/exec.c                   |  9 +++++++++
 gdb/exec.h                   |  5 +++++
 gdb/gnu-nat.c                |  3 +++
 gdb/go32-nat.c               |  6 ++----
 gdb/inf-ptrace.c             |  3 +++
 gdb/nat/fork-inferior.c      | 16 +++++-----------
 gdb/nto-procfs.c             |  7 +++++--
 gdb/procfs.c                 |  3 +++
 gdb/symfile.c                |  4 +++-
 gdbserver/server.cc          | 11 -----------
 gdbsupport/common-inferior.h |  5 -----
 13 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/gdb/corefile.c b/gdb/corefile.c
index aa35e3aa4e1f..96052cfdc6d4 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -76,19 +76,6 @@ validate_files (void)
     }
 }
 
-/* See gdbsupport/common-inferior.h.  */
-
-const char *
-get_exec_file ()
-{
-  if (current_program_space->exec_filename () != nullptr)
-    return current_program_space->exec_filename ();
-
-  error (_("No executable file specified.\n\
-Use the \"file\" or \"exec-file\" command."));
-}
-\f
-
 std::string
 memory_error_message (enum target_xfer_status err,
 		      struct gdbarch *gdbarch, CORE_ADDR memaddr)
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index c70cd44bb7fb..d1bcf940dd11 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1968,6 +1968,9 @@ darwin_nat_target::create_inferior (const char *exec_file,
 				    const std::string &allargs,
 				    char **env, int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   std::optional<scoped_restore_tmpl<bool>> restore_startup_with_shell;
   darwin_nat_target *the_target = this;
 
diff --git a/gdb/exec.c b/gdb/exec.c
index 496f3b130708..63eee4297bc4 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -504,6 +504,15 @@ exec_file_attach (const char *filename, int from_tty)
   gdb::observers::executable_changed.notify (current_program_space, reload_p);
 }
 
+/* See exec.h.  */
+
+void
+no_executable_specified_error ()
+{
+  error (_("No executable file specified.\n\
+Use the \"file\" or \"exec-file\" command."));
+}
+
 /*  Process the first arg in ARGS as the new exec file.
 
    Note that we have to explicitly ignore additional args, since we can
diff --git a/gdb/exec.h b/gdb/exec.h
index f667f23600d8..0c1f60495d72 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -105,4 +105,9 @@ extern void print_section_info (const std::vector<target_section> *table,
 extern void try_open_exec_file (const char *exec_file_host,
 				struct inferior *inf,
 				symfile_add_flags add_flags);
+
+/* Report a "No executable file specified" error.  */
+
+extern void no_executable_specified_error ();
+
 #endif
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 198fc42c2178..1d3e523a9631 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2103,6 +2103,9 @@ gnu_nat_target::create_inferior (const char *exec_file,
 				 char **env,
 				 int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   struct inf *inf = cur_inf ();
   inferior *inferior = current_inferior ();
   int pid;
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index f59a7ed5a2bc..84533669b53b 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -681,10 +681,8 @@ go32_nat_target::create_inferior (const char *exec_file,
   int result;
   const char *args = allargs.c_str ();
 
-  /* If no exec file handed to us, get it from the exec-file command -- with
-     a good, common error message if none is specified.  */
-  if (exec_file == 0)
-    exec_file = get_exec_file ();
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
 
   resume_signal = -1;
   resume_is_step = 0;
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index ce303eb87eaf..acb80af9bff1 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -75,6 +75,9 @@ inf_ptrace_target::create_inferior (const char *exec_file,
 				    const std::string &allargs,
 				    char **env, int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   inferior *inf = current_inferior ();
 
   /* Do not change either targets above or the same target if already present.
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 172eef0f93b0..2fd9cba52ee0 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -265,26 +265,20 @@ execv_argv::init_for_shell (const char *exec_file,
 /* See nat/fork-inferior.h.  */
 
 pid_t
-fork_inferior (const char *exec_file_arg, const std::string &allargs,
-	       char **env, traceme_ftype traceme_fun,
-	       init_trace_ftype init_trace_fun, pre_trace_ftype pre_trace_fun,
-	       const char *shell_file_arg, exec_ftype exec_fun)
+fork_inferior (const char *exec_file, const std::string &allargs, char **env,
+	       traceme_ftype traceme_fun, init_trace_ftype init_trace_fun,
+	       pre_trace_ftype pre_trace_fun, const char *shell_file_arg,
+	       exec_ftype exec_fun)
 {
   pid_t pid;
   /* Set debug_fork then attach to the child while it sleeps, to debug.  */
   int debug_fork = 0;
   const char *shell_file;
-  const char *exec_file;
   char **save_our_env;
   int i;
   int save_errno;
 
-  /* If no exec file handed to us, get it from the exec-file command
-     -- with a good, common error message if none is specified.  */
-  if (exec_file_arg == NULL)
-    exec_file = get_exec_file ();
-  else
-    exec_file = exec_file_arg;
+  gdb_assert (exec_file != nullptr);
 
   /* 'startup_with_shell' is declared in inferior.h and bound to the
      "set startup-with-shell" option.  If 0, we'll just do a
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 9ec214771271..c3108746d946 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -1179,6 +1179,9 @@ nto_procfs_target::create_inferior (const char *exec_file,
 				    const std::string &allargs,
 				    char **env, int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   struct inheritance inherit;
   pid_t pid;
   int flags, errn;
@@ -1190,9 +1193,9 @@ nto_procfs_target::create_inferior (const char *exec_file,
 
   argv = xmalloc ((allargs.size () / (unsigned) 2 + 2) *
 		  sizeof (*argv));
-  argv[0] = const_cast<char *> (get_exec_file ());
+  argv[0] = exec_file;
   args = xstrdup (allargs.c_str ());
-  breakup_args (args, (exec_file != NULL) ? &argv[1] : &argv[0]);
+  breakup_args (args, &argv[1]);
 
   argv = nto_parse_redirection (argv, &in, &out, &err);
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 93d1113893d2..a9a26c6b94cc 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -2760,6 +2760,9 @@ procfs_target::create_inferior (const char *exec_file,
 				const std::string &allargs,
 				char **env, int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   const char *shell_file = get_shell ();
   char *tryname;
   int pid;
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 78e68139c6f7..a49ef9f24303 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1843,7 +1843,9 @@ load_command (const char *arg, int from_tty)
     {
       const char *parg, *prev;
 
-      arg = get_exec_file ();
+      arg = current_program_space->exec_filename ();
+      if (arg == nullptr)
+	no_executable_specified_error ();
 
       /* We may need to quote this string so buildargv can pull it
 	 apart.  */
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 82f4318457cb..6f9d2a85609b 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -282,17 +282,6 @@ get_exec_wrapper ()
   return !wrapper_argv.empty () ? wrapper_argv.c_str () : NULL;
 }
 
-/* See gdbsupport/common-inferior.h.  */
-
-const char *
-get_exec_file ()
-{
-  if (program_path.get () == NULL)
-    error (_("No executable file specified."));
-
-  return program_path.get ();
-}
-
 /* See server.h.  */
 
 gdb_environ *
diff --git a/gdbsupport/common-inferior.h b/gdbsupport/common-inferior.h
index 9c19e9d9fdd9..cc625aec2368 100644
--- a/gdbsupport/common-inferior.h
+++ b/gdbsupport/common-inferior.h
@@ -27,11 +27,6 @@
    otherwise.  */
 extern const char *get_exec_wrapper ();
 
-/* Return the name of the executable file as a string.
-
-   Error out if no executable is specified.  */
-extern const char *get_exec_file ();
-
 /* Return the inferior's current working directory.
 
    If it is not set, the string is empty.  */
-- 
2.45.1

Re: [PATCH 0/4] Cleanups, remove get_exec_file

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> Some cleanups and simplifications around getting the exec filename.  No
Simon> behavior change intended.

This all looks reasonable to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 0/4] Cleanups, remove get_exec_file

[Simon Marchi]

On 2024-06-03 10:53, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> Some cleanups and simplifications around getting the exec filename.  No
> Simon> behavior change intended.
> 
> This all looks reasonable to me.
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom

Thanks, pushed.

Simon