[PATCH 0/2] Unify target announce and detach messages

[PATCH 0/2] Unify target announce and detach messages

[Tom Tromey]

While slogging through all the printf's in gdb, I found some repeated
code in the target layers to announce attach or detaches.

This series unifies this code and removes the redundancies.

I wasn't able to build some of these, as I don't have the proper
operating systems.  So, those are on a "best effort" basis.

Tom

[PATCH 1/2] Introduce target_announce_attach

[Tom Tromey]

This introduces target_announce_attach, by analog with
target_announce_detach.  Then it converts existing targets to use
this, rather than emitting their own output by hand.
---
 gdb/darwin-nat.c  | 12 +-----------
 gdb/gnu-nat.c     | 11 +----------
 gdb/inf-ptrace.c  | 12 +-----------
 gdb/nto-procfs.c  | 11 +----------
 gdb/procfs.c      | 14 +-------------
 gdb/remote.c      | 12 +-----------
 gdb/target.c      | 18 ++++++++++++++++++
 gdb/target.h      |  5 +++++
 gdb/windows-nat.c | 12 +-----------
 9 files changed, 30 insertions(+), 77 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 98720b330aa..e9c24dcc5dc 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -2002,17 +2002,7 @@ darwin_nat_target::attach (const char *args, int from_tty)
   if (pid == getpid ())		/* Trying to masturbate?  */
     error (_("I refuse to debug myself!"));
 
-  if (from_tty)
-    {
-      const char *exec_file = get_exec_file (0);
-
-      if (exec_file)
-	printf_unfiltered (_("Attaching to program: %s, %s\n"), exec_file,
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-      else
-	printf_unfiltered (_("Attaching to %s\n"),
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-    }
+  target_announce_attach (from_tty, pid);
 
   if (pid == 0 || ::kill (pid, 0) < 0)
     error (_("Can't attach to process %d: %s (%d)"),
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 4b22e21bb8e..ad870c94d63 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2170,16 +2170,7 @@ gnu_nat_target::attach (const char *args, int from_tty)
   if (pid == getpid ())		/* Trying to masturbate?  */
     error (_("I refuse to debug myself!"));
 
-  if (from_tty)
-    {
-      const char *exec_file = get_exec_file (0);
-
-      if (exec_file)
-	printf_unfiltered ("Attaching to program `%s', pid %d\n",
-			   exec_file, pid);
-      else
-	printf_unfiltered ("Attaching to pid %d\n", pid);
-    }
+  target_announce_attach (from_tty, pid);
 
   inf_debug (inf, "attaching to pid: %d", pid);
 
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 521b41c6ea6..6e4706a3d20 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -148,17 +148,7 @@ inf_ptrace_target::attach (const char *args, int from_tty)
       unpusher.reset (this);
     }
 
-  if (from_tty)
-    {
-      const char *exec_file = get_exec_file (0);
-
-      if (exec_file)
-	printf_unfiltered (_("Attaching to program: %s, %s\n"), exec_file,
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-      else
-	printf_unfiltered (_("Attaching to %s\n"),
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-    }
+  target_announce_attach (from_tty, pid);
 
 #ifdef PT_ATTACH
   errno = 0;
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 7ae71d9d519..e27da7ca2d2 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -701,17 +701,8 @@ nto_procfs_target::attach (const char *args, int from_tty)
   if (pid == getpid ())
     error (_("Attaching GDB to itself is not a good idea..."));
 
-  if (from_tty)
-    {
-      const char *exec_file = get_exec_file (0);
+  target_announce_attach (from_tty, pid);
 
-      if (exec_file)
-	printf_unfiltered ("Attaching to program `%s', %s\n", exec_file,
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-      else
-	printf_unfiltered ("Attaching to %s\n",
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-    }
   ptid_t ptid = do_attach (ptid_t (pid));
   inf = current_inferior ();
   inferior_appeared (inf, pid);
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 201e37c3a4b..ddc8623b1e1 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -1775,19 +1775,7 @@ procfs_target::attach (const char *args, int from_tty)
       unpusher.reset (this);
     }
 
-  if (from_tty)
-    {
-      const char *exec_file = get_exec_file (0);
-
-      if (exec_file)
-	printf_filtered (_("Attaching to program `%s', %s\n"),
-			 exec_file, target_pid_to_str (ptid_t (pid)).c_str ());
-      else
-	printf_filtered (_("Attaching to %s\n"),
-			 target_pid_to_str (ptid_t (pid)).c_str ());
-
-      fflush (stdout);
-    }
+  target_announce_attach (from_tty, pid);
 
   do_attach (ptid_t (pid));
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 98003d39918..290edd07cd1 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6115,17 +6115,7 @@ extended_remote_target::attach (const char *args, int from_tty)
   if (packet_support (PACKET_vAttach) == PACKET_DISABLE)
     error (_("This target does not support attaching to a process"));
 
-  if (from_tty)
-    {
-      const char *exec_file = get_exec_file (0);
-
-      if (exec_file)
-	printf_unfiltered (_("Attaching to program: %s, %s\n"), exec_file,
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-      else
-	printf_unfiltered (_("Attaching to %s\n"),
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-    }
+  target_announce_attach (from_tty, pid);
 
   xsnprintf (rs->buf.data (), get_remote_packet_size (), "vAttach;%x", pid);
   putpkt (rs->buf);
diff --git a/gdb/target.c b/gdb/target.c
index e10b295c797..65b98c5a9b8 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3637,6 +3637,24 @@ target_announce_detach (int from_tty)
 		     target_pid_to_str (ptid_t (pid)).c_str ());
 }
 
+/* See target.h  */
+
+void
+target_announce_attach (int from_tty, int pid)
+{
+  if (!from_tty)
+    return;
+
+  const char *exec_file = get_exec_file (0);
+
+  if (exec_file)
+    printf_unfiltered ("Attaching to program: %s, %s\n", exec_file,
+		       target_pid_to_str (ptid_t (pid)).c_str ());
+  else
+    printf_unfiltered ("Attaching to %s\n",
+		       target_pid_to_str (ptid_t (pid)).c_str ());
+}
+
 /* The inferior process has died.  Long live the inferior!  */
 
 void
diff --git a/gdb/target.h b/gdb/target.h
index c629b9ba3fb..1ac7a4554dc 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1448,6 +1448,11 @@ extern bool target_attach_no_wait ();
 
 extern void target_post_attach (int pid);
 
+/* Display a message indicating we're about to attach to a given
+   process.  */
+
+extern void target_announce_attach (int from_tty, int pid);
+
 /* Display a message indicating we're about to detach from the current
    inferior process.  */
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 76332541f8e..89084acabaa 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1932,17 +1932,7 @@ windows_nat_target::attach (const char *args, int from_tty)
 
   DebugSetProcessKillOnExit (FALSE);
 
-  if (from_tty)
-    {
-      const char *exec_file = get_exec_file (0);
-
-      if (exec_file)
-	printf_unfiltered ("Attaching to program `%s', %s\n", exec_file,
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-      else
-	printf_unfiltered ("Attaching to %s\n",
-			   target_pid_to_str (ptid_t (pid)).c_str ());
-    }
+  target_announce_attach (from_tty, pid);
 
 #ifdef __x86_64__
   HANDLE h = OpenProcess (PROCESS_QUERY_INFORMATION, FALSE, pid);
-- 
2.31.1

[PATCH 2/2] Use target_announce_detach in more targets

[Tom Tromey]

target_announce_detach was added in commit 0f48b757 ("Factor out
"Detaching from program" message printing").  There, Pedro wrote:

    (For now, I left the couple targets that print this a bit differently
    alone.  Maybe this could be further pulled out into infcmd.c.  If we
    did that, and those targets want to continue printing differently,
    this new function could be converted to a target method.)

It seems to me that the differences aren't very big, and in some cases
other targets handled the output a bit more nicely.  In particular,
some targets will print a different message when exec_file==NULL,
rather than printing the same output with an empty string as
exec_file.

This patch incorporates the nicer output into target_announce_detach,
then changes the remaining ports to use this function.
---
 gdb/gnu-nat.c     | 11 +----------
 gdb/procfs.c      | 14 +-------------
 gdb/target.c      | 13 +++++++------
 gdb/windows-nat.c | 10 ++--------
 4 files changed, 11 insertions(+), 37 deletions(-)

diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index ad870c94d63..b6bccd8b4b7 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2215,16 +2215,7 @@ gnu_nat_target::attach (const char *args, int from_tty)
 void
 gnu_nat_target::detach (inferior *inf, int from_tty)
 {
-  if (from_tty)
-    {
-      const char *exec_file = get_exec_file (0);
-
-      if (exec_file)
-	printf_unfiltered ("Detaching from program `%s' pid %d\n",
-			   exec_file, gnu_current_inf->pid);
-      else
-	printf_unfiltered ("Detaching from pid %d\n", gnu_current_inf->pid);
-    }
+  target_announce_detach (from_tty);
 
   inf_detach (gnu_current_inf);
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index ddc8623b1e1..840201d1897 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -1786,19 +1786,7 @@ procfs_target::attach (const char *args, int from_tty)
 void
 procfs_target::detach (inferior *inf, int from_tty)
 {
-  int pid = inferior_ptid.pid ();
-
-  if (from_tty)
-    {
-      const char *exec_file;
-
-      exec_file = get_exec_file (0);
-      if (exec_file == NULL)
-	exec_file = "";
-
-      printf_filtered (_("Detaching from program: %s, %s\n"), exec_file,
-		       target_pid_to_str (ptid_t (pid)).c_str ());
-    }
+  target_announce_detach (from_tty);
 
   do_detach ();
 
diff --git a/gdb/target.c b/gdb/target.c
index 65b98c5a9b8..90a279b2902 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3628,13 +3628,14 @@ target_announce_detach (int from_tty)
   if (!from_tty)
     return;
 
-  exec_file = get_exec_file (0);
-  if (exec_file == NULL)
-    exec_file = "";
-
   pid = inferior_ptid.pid ();
-  printf_unfiltered (_("Detaching from program: %s, %s\n"), exec_file,
-		     target_pid_to_str (ptid_t (pid)).c_str ());
+  exec_file = get_exec_file (0);
+  if (exec_file == nullptr)
+    printf_unfiltered ("Detaching from pid %s\n",
+		       target_pid_to_str (ptid_t (pid)).c_str ());
+  else
+    printf_unfiltered (_("Detaching from program: %s, %s\n"), exec_file,
+		       target_pid_to_str (ptid_t (pid)).c_str ());
 }
 
 /* See target.h  */
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 89084acabaa..0faf138c0c7 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1965,14 +1965,8 @@ windows_nat_target::detach (inferior *inf, int from_tty)
     }
   DebugSetProcessKillOnExit (FALSE);
 
-  if (detached && from_tty)
-    {
-      const char *exec_file = get_exec_file (0);
-      if (exec_file == 0)
-	exec_file = "";
-      printf_unfiltered ("Detaching from program: %s, Pid %u\n", exec_file,
-			 (unsigned) current_event.dwProcessId);
-    }
+  if (detached)
+    target_announce_detach (from_tty);
 
   x86_cleanup_dregs ();
   switch_to_no_thread ();
-- 
2.31.1

Re: [PATCH 0/2] Unify target announce and detach messages

[John Baldwin]

On 1/5/22 9:59 AM, Tom Tromey wrote:
> While slogging through all the printf's in gdb, I found some repeated
> code in the target layers to announce attach or detaches.
> 
> This series unifies this code and removes the redundancies.
> 
> I wasn't able to build some of these, as I don't have the proper
> operating systems.  So, those are on a "best effort" basis.

Looks good to me.

-- 
John Baldwin

Re: [PATCH 0/2] Unify target announce and detach messages

[Joel Brobecker]

Hi Tom,

On Wed, Jan 05, 2022 at 10:59:14AM -0700, Tom Tromey wrote:
> While slogging through all the printf's in gdb, I found some repeated
> code in the target layers to announce attach or detaches.
> 
> This series unifies this code and removes the redundancies.
> 
> I wasn't able to build some of these, as I don't have the proper
> operating systems.  So, those are on a "best effort" basis.

Thanks for those patches. These are a nice little refactoring.
I agree with John that these look good.

-- 
Joel

Re: [PATCH 1/2] Introduce target_announce_attach

[Lancelot SIX]

> +/* See target.h  */
> +
> +void
> +target_announce_attach (int from_tty, int pid)
> +{
> +  if (!from_tty)
> +    return;
> +
> +  const char *exec_file = get_exec_file (0);
> +
> +  if (exec_file)

Hi,

Just a nit, and I know you just moved this bit of code around,  but
shouldn't this become

    if (exec_file != nullptr)

?

Lancelot.

> +    printf_unfiltered ("Attaching to program: %s, %s\n", exec_file,
> +		       target_pid_to_str (ptid_t (pid)).c_str ());
> +  else
> +    printf_unfiltered ("Attaching to %s\n",
> +		       target_pid_to_str (ptid_t (pid)).c_str ());
> +}
> +
>  /* The inferior process has died.  Long live the inferior!  */

Re: [PATCH 1/2] Introduce target_announce_attach

[Tom Tromey]

Lancelot> Just a nit, and I know you just moved this bit of code around,  but
Lancelot> shouldn't this become

Lancelot>     if (exec_file != nullptr)

Lancelot> ?

Yes, thanks.  I'll send a patch to fix it shortly.

Tom