[PATCH 1/2] linux-nat: Remove unnecessary xstrdup

[PATCH 1/2] linux-nat: Remove unnecessary xstrdup

[Simon Marchi]

I think this xstrdup is not useful.  We can pass ex.message directly to
throw_error instead.

gdb/ChangeLog:

	* linux-nat.c (linux_nat_attach): Remove xstrdup.
---
 gdb/linux-nat.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b8f3108..1d1d6d3 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1224,10 +1224,7 @@ linux_nat_attach (struct target_ops *ops, const char *args, int from_tty)
     {
       pid_t pid = parse_pid_to_attach (args);
       struct buffer buffer;
-      char *message, *buffer_s;
-
-      message = xstrdup (ex.message);
-      make_cleanup (xfree, message);
+      char *buffer_s;
 
       buffer_init (&buffer);
       linux_ptrace_attach_fail_reason (pid, &buffer);
@@ -1237,9 +1234,9 @@ linux_nat_attach (struct target_ops *ops, const char *args, int from_tty)
       make_cleanup (xfree, buffer_s);
 
       if (*buffer_s != '\0')
-	throw_error (ex.error, "warning: %s\n%s", buffer_s, message);
+	throw_error (ex.error, "warning: %s\n%s", buffer_s, ex.message);
       else
-	throw_error (ex.error, "%s", message);
+	throw_error (ex.error, "%s", ex.message);
     }
   END_CATCH
 
-- 
2.7.4

[PATCH 2/2] Make linux_ptrace_attach_fail_reason return an std::string

[Simon Marchi]

This patch makes linux_ptrace_attach_fail_reason and
linux_ptrace_attach_fail_reason_string return std::string.  It also
replaces usages of struct buffer with std::string.  This allows getting
rid of a cleanup in in linux_ptrace_attach_fail_reason_string and
simplifies the code in general.

Something that looks odd to me is that in
linux_ptrace_attach_fail_reason, if the two messages are appended, there
is no separate space or \n, so the result won't be very nice.  I left it
as-is for now though.

gdb/ChangeLog:

	* nat/linux-ptrace.h (linux_ptrace_attach_fail_reason): Return
	std::string.
	(linux_ptrace_attach_fail_reason_string): Likewise.
	* nat/linux-ptrace.c (linux_ptrace_attach_fail_reason):
	Likewise.
	(linux_ptrace_attach_fail_reason_string): Likewise.
	* linux-nat.c (attach_proc_task_lwp_callback): Adjust.

gdb/gdbserver/ChangeLog:

	* linux-low.c (attach_proc_task_lwp_callback): Adjust to
	linux_ptrace_attach_fail_reason_string now returning an
	std::string.
	(linux_attach): Likewise.
	* thread-db.c (attach_thread): Likewise.
---
 gdb/gdbserver/linux-low.c | 14 +++++++-----
 gdb/gdbserver/thread-db.c |  6 ++++--
 gdb/linux-nat.c           | 21 ++++++------------
 gdb/nat/linux-ptrace.c    | 55 ++++++++++++++++++++---------------------------
 gdb/nat/linux-ptrace.h    |  8 +++----
 5 files changed, 46 insertions(+), 58 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index f6a52d5..711d9e6 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -1159,9 +1159,10 @@ attach_proc_task_lwp_callback (ptid_t ptid)
 	}
       else if (err != 0)
 	{
-	  warning (_("Cannot attach to lwp %d: %s"),
-		   lwpid,
-		   linux_ptrace_attach_fail_reason_string (ptid, err));
+	  std::string reason
+	    = linux_ptrace_attach_fail_reason_string (ptid, err);
+
+	  warning (_("Cannot attach to lwp %d: %s"), lwpid, reason.c_str ());
 	}
 
       return 1;
@@ -1186,8 +1187,11 @@ linux_attach (unsigned long pid)
      soon.  */
   err = linux_attach_lwp (ptid);
   if (err != 0)
-    error ("Cannot attach to process %ld: %s",
-	   pid, linux_ptrace_attach_fail_reason_string (ptid, err));
+    {
+      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
+
+      error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
+    }
 
   proc = linux_add_process (pid, 1);
 
diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index 537758c..1c377ae 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -225,9 +225,11 @@ attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p)
   err = linux_attach_lwp (ptid);
   if (err != 0)
     {
+      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
+
       warning ("Could not attach to thread %ld (LWP %d): %s\n",
-	       (unsigned long) ti_p->ti_tid, ti_p->ti_lid,
-	       linux_ptrace_attach_fail_reason_string (ptid, err));
+	       (unsigned long) ti_p->ti_tid, ti_p->ti_lid, reason.c_str ());
+
       return 0;
     }
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 1d1d6d3..ef5a58e 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1167,10 +1167,11 @@ attach_proc_task_lwp_callback (ptid_t ptid)
 	    }
 	  else
 	    {
+	      std::string reason
+		= linux_ptrace_attach_fail_reason_string (ptid, err);
+
 	      warning (_("Cannot attach to lwp %d: %s"),
-		       lwpid,
-		       linux_ptrace_attach_fail_reason_string (ptid,
-							       err));
+		       lwpid, reason.c_str ());
 	    }
 	}
       else
@@ -1223,18 +1224,10 @@ linux_nat_attach (struct target_ops *ops, const char *args, int from_tty)
   CATCH (ex, RETURN_MASK_ERROR)
     {
       pid_t pid = parse_pid_to_attach (args);
-      struct buffer buffer;
-      char *buffer_s;
-
-      buffer_init (&buffer);
-      linux_ptrace_attach_fail_reason (pid, &buffer);
-
-      buffer_grow_str0 (&buffer, "");
-      buffer_s = buffer_finish (&buffer);
-      make_cleanup (xfree, buffer_s);
+      std::string reason = linux_ptrace_attach_fail_reason (pid);
 
-      if (*buffer_s != '\0')
-	throw_error (ex.error, "warning: %s\n%s", buffer_s, ex.message);
+      if (!reason.empty ())
+	throw_error (ex.error, "warning: %s\n%s", reason.c_str (), ex.message);
       else
 	throw_error (ex.error, "%s", ex.message);
     }
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index 438177f..2533b6c 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -32,51 +32,42 @@
    of 0 means there are no supported features.  */
 static int supported_ptrace_options = -1;
 
-/* Find all possible reasons we could fail to attach PID and append
-   these as strings to the already initialized BUFFER.  '\0'
-   termination of BUFFER must be done by the caller.  */
+/* Find all possible reasons we could fail to attach PID and return these
+   as a string.  An empty string is returned if we didn't find any reason.  */
 
-void
-linux_ptrace_attach_fail_reason (pid_t pid, struct buffer *buffer)
+std::string
+linux_ptrace_attach_fail_reason (pid_t pid)
 {
-  pid_t tracerpid;
+  pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid);
+  std::string result;
 
-  tracerpid = linux_proc_get_tracerpid_nowarn (pid);
   if (tracerpid > 0)
-    buffer_xml_printf (buffer, _("process %d is already traced "
-				 "by process %d"),
-		       (int) pid, (int) tracerpid);
+    string_appendf (result,
+		    _("process %d is already traced by process %d"),
+		    (int) pid, (int) tracerpid);
 
   if (linux_proc_pid_is_zombie_nowarn (pid))
-    buffer_xml_printf (buffer, _("process %d is a zombie "
-				 "- the process has already terminated"),
-		       (int) pid);
+    string_appendf (result,
+		    _("process %d is a zombie - the process has already "
+		      "terminated"),
+		    (int) pid);
+
+  return result;
 }
 
 /* See linux-ptrace.h.  */
 
-char *
+std::string
 linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err)
 {
-  static char *reason_string;
-  struct buffer buffer;
-  char *warnings;
-  long lwpid = ptid_get_lwp (ptid);
-
-  xfree (reason_string);
-
-  buffer_init (&buffer);
-  linux_ptrace_attach_fail_reason (lwpid, &buffer);
-  buffer_grow_str0 (&buffer, "");
-  warnings = buffer_finish (&buffer);
-  if (warnings[0] != '\0')
-    reason_string = xstrprintf ("%s (%d), %s",
-				safe_strerror (err), err, warnings);
+  long lwpid = ptid.lwp ();
+  std::string reason = linux_ptrace_attach_fail_reason (lwpid);
+
+  if (!reason.empty ())
+    return string_printf ("%s (%d), %s", safe_strerror (err), err,
+			  reason.c_str ());
   else
-    reason_string = xstrprintf ("%s (%d)",
-				safe_strerror (err), err);
-  xfree (warnings);
-  return reason_string;
+    return string_printf ("%s (%d)", safe_strerror (err), err);
 }
 
 #if defined __i386__ || defined __x86_64__
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 8a8c4c6..eab4154 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -180,14 +180,12 @@ struct buffer;
 # define TRAP_HWBKPT 4
 #endif
 
-extern void linux_ptrace_attach_fail_reason (pid_t pid, struct buffer *buffer);
+extern std::string linux_ptrace_attach_fail_reason (pid_t pid);
 
 /* Find all possible reasons we could have failed to attach to PTID
    and return them as a string.  ERR is the error PTRACE_ATTACH failed
-   with (an errno).  The result is stored in a static buffer.  This
-   string should be copied into a buffer by the client if the string
-   will not be immediately used, or if it must persist.  */
-extern char *linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err);
+   with (an errno).  */
+extern std::string linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err);
 
 extern void linux_ptrace_init_warnings (void);
 extern void linux_check_ptrace_features (void);
-- 
2.7.4

Re: [PATCH 1/2] linux-nat: Remove unnecessary xstrdup

[Pedro Alves]

On 12/31/2017 02:59 PM, Simon Marchi wrote:
> I think this xstrdup is not useful.  We can pass ex.message directly to
> throw_error instead.
> 
> gdb/ChangeLog:
> 
> 	* linux-nat.c (linux_nat_attach): Remove xstrdup.

LGTM.

Thanks,
Pedro Alves

Re: [PATCH 2/2] Make linux_ptrace_attach_fail_reason return an std::string

[Pedro Alves]

On 12/31/2017 02:59 PM, Simon Marchi wrote:
> This patch makes linux_ptrace_attach_fail_reason and
> linux_ptrace_attach_fail_reason_string return std::string.  It also
> replaces usages of struct buffer with std::string.  This allows getting
> rid of a cleanup in in linux_ptrace_attach_fail_reason_string and
> simplifies the code in general.
> 
> Something that looks odd to me is that in
> linux_ptrace_attach_fail_reason, if the two messages are appended, there
> is no separate space or \n, so the result won't be very nice.  I left it
> as-is for now though.
> 
> gdb/ChangeLog:
> 
> 	* nat/linux-ptrace.h (linux_ptrace_attach_fail_reason): Return
> 	std::string.
> 	(linux_ptrace_attach_fail_reason_string): Likewise.
> 	* nat/linux-ptrace.c (linux_ptrace_attach_fail_reason):
> 	Likewise.
> 	(linux_ptrace_attach_fail_reason_string): Likewise.
> 	* linux-nat.c (attach_proc_task_lwp_callback): Adjust.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* linux-low.c (attach_proc_task_lwp_callback): Adjust to
> 	linux_ptrace_attach_fail_reason_string now returning an
> 	std::string.
> 	(linux_attach): Likewise.
> 	* thread-db.c (attach_thread): Likewise.

LGTM.

Thanks,
Pedro Alves

Re: [PATCH 1/2] linux-nat: Remove unnecessary xstrdup

[Simon Marchi]

On 2018-01-17 07:25, Pedro Alves wrote:
> On 12/31/2017 02:59 PM, Simon Marchi wrote:
>> I think this xstrdup is not useful.  We can pass ex.message directly 
>> to
>> throw_error instead.
>> 
>> gdb/ChangeLog:
>> 
>> 	* linux-nat.c (linux_nat_attach): Remove xstrdup.
> 
> LGTM.
> 
> Thanks,
> Pedro Alves

Thanks, I pushed both patches.

Simon