]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdb: reorder checks in validate_exec_file
authorAndrew Burgess <aburgess@redhat.com>
Wed, 21 May 2025 09:16:08 +0000 (10:16 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Thu, 22 May 2025 18:14:01 +0000 (19:14 +0100)
While reviewing another patch I was briefly confused by a call to
target_pid_to_exec_file coming from validate_exec_file while attaching
to a process when I had not previously set an executable.

The current order of actions in validate_exec_file is:

  1. Get name of current executable.
  2. Get name of executable from the current inferior.
  3. If either of (1) or (2) return NULL, then there's nothing to
     check, early return.

I think it would be cleaner if we instead did this:

  1. Get name of current executable.
  3. If (1) returned NULL then there's nothing to check, early return.
  3. Get name of executable from the current inferior.
  4. If (3) returned NULL then there's nothing to check, early return.

This does mean there's an extra step, but I don't think the code is
any more complex really, and we now avoid trying to extract the name
of the executable from the current inferior unless we really need it.
This avoids the target_pid_to_exec_file call that I was seeing, which
for remote targets does avoid a packet round trip (not that I'm
selling this as an "optimisation", just noting the change).

There should be no user visible changes after this commit.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
gdb/exec.c

index c7979a2bd94f2dae94671a58445e7544215d613b..33183ab2de80e184919d09113a0d95c21944d733 100644 (file)
@@ -218,28 +218,32 @@ validate_exec_file (int from_tty)
   if (exec_file_mismatch_mode == exec_file_mismatch_off)
     return;
 
+  /* If there's no current executable, then there's nothing to
+     validate against, so we're done.  */
   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);
-  bool build_id_mismatch = false;
-
-  /* If we cannot validate the exec file, return.  */
-  if (current_exec_file == NULL || pid_exec_file == NULL)
+  if (current_exec_file == nullptr)
     return;
 
-  /* Try validating via build-id, if available.  This is the most
-     reliable check.  */
+  /* Try to determine a filename from the process itself.  If we
+     cannot get an executable from the process, then no validation is
+     possible.  */
+  const char *pid_exec_file
+    = target_pid_to_exec_file (current_inferior ()->pid);
+  if (pid_exec_file == nullptr)
+    return;
 
-  /* In case current_exec_file was changed, reopen_exec_file ensures
-     an up to date build_id (will do nothing if the file timestamp
-     did not change).  If exec file changed, reopen_exec_file has
-     allocated another file name, so get_exec_file again.  */
+  /* In case current_exec_file was changed, reopen_exec_file ensures an up
+     to date build_id (will do nothing if the file timestamp 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 = current_program_space->exec_filename ();
 
+  /* Try validating via build-id, if available.  This is the most reliable
+     check.  */
   const bfd_build_id *exec_file_build_id
     = build_id_bfd_get (current_program_space->exec_bfd ());
+  bool build_id_mismatch = false;
   if (exec_file_build_id != nullptr)
     {
       /* Prepend the target prefix, to force gdb_bfd_open to open the