* [PATCH 1/4] gdb: make progspace::exec_filename private, add getter / setter
2024-05-30 18:53 [PATCH 0/4] Cleanups, remove get_exec_file Simon Marchi
@ 2024-05-30 18:53 ` Simon Marchi
2024-05-30 18:53 ` [PATCH 2/4] gdb: replace `get_exec_file (0)` calls with `current_program_space->exec_filename ()` Simon Marchi
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi @ 2024-05-30 18:53 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 2/4] gdb: replace `get_exec_file (0)` calls with `current_program_space->exec_filename ()`
2024-05-30 18:53 [PATCH 0/4] Cleanups, remove get_exec_file Simon Marchi
2024-05-30 18:53 ` [PATCH 1/4] gdb: make progspace::exec_filename private, add getter / setter Simon Marchi
@ 2024-05-30 18:53 ` Simon Marchi
2024-05-30 18:53 ` [PATCH 3/4] gdb: remove dead code in nto-procfs.c Simon Marchi
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi @ 2024-05-30 18:53 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 3/4] gdb: remove dead code in nto-procfs.c
2024-05-30 18:53 [PATCH 0/4] Cleanups, remove get_exec_file Simon Marchi
2024-05-30 18:53 ` [PATCH 1/4] gdb: make progspace::exec_filename private, add getter / setter Simon Marchi
2024-05-30 18:53 ` [PATCH 2/4] gdb: replace `get_exec_file (0)` calls with `current_program_space->exec_filename ()` Simon Marchi
@ 2024-05-30 18:53 ` Simon Marchi
2024-05-30 18:53 ` [PATCH 4/4] gdb: remove get_exec_file Simon Marchi
2024-06-03 14:53 ` [PATCH 0/4] Cleanups, " Tom Tromey
4 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi @ 2024-05-30 18:53 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 4/4] gdb: remove get_exec_file
2024-05-30 18:53 [PATCH 0/4] Cleanups, remove get_exec_file Simon Marchi
` (2 preceding siblings ...)
2024-05-30 18:53 ` [PATCH 3/4] gdb: remove dead code in nto-procfs.c Simon Marchi
@ 2024-05-30 18:53 ` Simon Marchi
2024-06-03 14:53 ` [PATCH 0/4] Cleanups, " Tom Tromey
4 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi @ 2024-05-30 18:53 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 0/4] Cleanups, remove get_exec_file
2024-05-30 18:53 [PATCH 0/4] Cleanups, remove get_exec_file Simon Marchi
` (3 preceding siblings ...)
2024-05-30 18:53 ` [PATCH 4/4] gdb: remove get_exec_file Simon Marchi
@ 2024-06-03 14:53 ` Tom Tromey
2024-06-08 3:16 ` Simon Marchi
4 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2024-06-03 14:53 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
>>>>> "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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 0/4] Cleanups, remove get_exec_file
2024-06-03 14:53 ` [PATCH 0/4] Cleanups, " Tom Tromey
@ 2024-06-08 3:16 ` Simon Marchi
0 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi @ 2024-06-08 3:16 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread