public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] display names of remote threads
@ 2015-05-14 22:26 Daniel Colascione
  2015-05-15  7:27 ` Eli Zaretskii
  2015-05-19 11:42 ` Pedro Alves
  0 siblings, 2 replies; 3+ messages in thread
From: Daniel Colascione @ 2015-05-14 22:26 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 10945 bytes --]

This patch adds an additional qXfer:threads:read attribute
for thread names.

commit 9b8ccc79e8a522b20ac6413751fa488622131839
Author: Daniel Colascione <dancol@dancol.org>
Date:   Thu May 14 15:24:08 2015 -0700

    Display names of remote threads

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4b76ce9..341ed82 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39111,17 +39111,19 @@ the following structure:
 @smallexample
 <?xml version="1.0"?>
 <threads>
-    <thread id="id" core="0">
+    <thread id="id" core="0" name="name">
     ... description ...
     </thread>
 </threads>
 @end smallexample

 Each @samp{thread} element must have the @samp{id} attribute that
-identifies the thread (@pxref{thread-id syntax}).  The
-@samp{core} attribute, if present, specifies which processor core
-the thread was last executing on.  The content of the of @samp{thread}
-element is interpreted as human-readable auxilliary information.
+identifies the thread (@pxref{thread-id syntax}).  The @samp{core}
+attribute, if present, specifies which processor core the thread was
+last executing on.  The @samp{name} attribute, if present, specifies
+the human-readable name of the thread.  The content of the of
+@samp{thread} element is interpreted as human-readable
+auxilliary information.

 @node Traceframe Info Format
 @section Traceframe Info Format
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index c6aa710..a72f862 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -6255,6 +6255,7 @@ static struct target_ops linux_target_ops = {
   NULL,
 #endif
   linux_supports_range_stepping,
+  linux_common_name_of_thread,
 };

 static void
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 83529ff..7dc99f1 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1355,20 +1355,23 @@ handle_qxfer_threads_worker (struct
inferior_list_entry *inf, void *arg)
   char ptid_s[100];
   int core = target_core_of_thread (ptid);
   char core_s[21];
+  char *name = NULL;

   write_ptid (ptid_s, ptid);

+  buffer_xml_printf (buffer, "<thread id=\"%s\"", ptid_s);
+
   if (core != -1)
     {
       sprintf (core_s, "%d", core);
-      buffer_xml_printf (buffer, "<thread id=\"%s\" core=\"%s\"/>\n",
-			 ptid_s, core_s);
-    }
-  else
-    {
-      buffer_xml_printf (buffer, "<thread id=\"%s\"/>\n",
-			 ptid_s);
+      buffer_xml_printf (buffer, " core=\"%s\"", core_s);
     }
+
+  name = target_thread_name (ptid);
+  if (name != NULL)
+    buffer_xml_printf (buffer, " name=\"%s\"", name);
+
+  buffer_xml_printf (buffer, "/>\n");
 }

 /* Helper for handle_qxfer_threads.  */
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 126c861..edc547c 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -394,6 +394,9 @@ struct target_ops

   /* Return true if target supports range stepping.  */
   int (*supports_range_stepping) (void);
+
+  /* Return name of thread if known.  */
+  char *(*thread_name) (ptid_t);
 };

 extern struct target_ops *the_target;
@@ -569,6 +572,10 @@ ptid_t mywait (ptid_t ptid, struct
target_waitstatus *ourstatus, int options,
   (the_target->core_of_thread ? (*the_target->core_of_thread) (ptid) \
    : -1)

+#define target_thread_name(ptid)                                \
+  (the_target->thread_name ? (*the_target->thread_name) (ptid)  \
+   : NULL)
+
 int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int
len);

 int write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 4a5a066..29fc340 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3924,38 +3924,7 @@ linux_nat_pid_to_str (struct target_ops *ops,
ptid_t ptid)
 static char *
 linux_nat_thread_name (struct target_ops *self, struct thread_info *thr)
 {
-  int pid = ptid_get_pid (thr->ptid);
-  long lwp = ptid_get_lwp (thr->ptid);
-#define FORMAT "/proc/%d/task/%ld/comm"
-  char buf[sizeof (FORMAT) + 30];
-  FILE *comm_file;
-  char *result = NULL;
-
-  snprintf (buf, sizeof (buf), FORMAT, pid, lwp);
-  comm_file = gdb_fopen_cloexec (buf, "r");
-  if (comm_file)
-    {
-      /* Not exported by the kernel, so we define it here.  */
-#define COMM_LEN 16
-      static char line[COMM_LEN + 1];
-
-      if (fgets (line, sizeof (line), comm_file))
-	{
-	  char *nl = strchr (line, '\n');
-
-	  if (nl)
-	    *nl = '\0';
-	  if (*line != '\0')
-	    result = line;
-	}
-
-      fclose (comm_file);
-    }
-
-#undef COMM_LEN
-#undef FORMAT
-
-  return result;
+  return linux_proc_tid_get_name (thr->ptid);
 }

 /* Accepts an integer PID; Returns a string representing a file that
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 0ed5d34..6cb01e6 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -34,6 +34,7 @@

 #include "xml-utils.h"
 #include "buffer.h"
+#include "linux-procfs.h"
 #include <dirent.h>
 #include <sys/stat.h>
 #include "filestuff.h"
@@ -109,6 +110,22 @@ linux_common_core_of_thread (ptid_t ptid)
   return core;
 }

+char *
+linux_common_name_of_thread (ptid_t ptid)
+{
+  static char buf[16 /*kernel maximum */ + 1];
+  char* name = linux_proc_tid_get_name (ptid);
+  if (name)
+    {
+      snprintf (buf, sizeof (buf), "%s", name);
+      free (name);
+    }
+  else
+    buf[0] = '\0';
+
+  return buf;
+}
+
 /* Finds the command-line of process PID and copies it into COMMAND.
    At most MAXLEN characters are copied.  If the command-line cannot
    be found, PID is copied into command in text-form.  */
diff --git a/gdb/nat/linux-osdata.h b/gdb/nat/linux-osdata.h
index db8b445..1fdf367 100644
--- a/gdb/nat/linux-osdata.h
+++ b/gdb/nat/linux-osdata.h
@@ -21,6 +21,7 @@
 #define COMMON_LINUX_OSDATA_H

 extern int linux_common_core_of_thread (ptid_t ptid);
+extern char *linux_common_name_of_thread (ptid_t ptid);
 extern LONGEST linux_common_xfer_osdata (const char *annex, gdb_byte
*readbuf,
 					 ULONGEST offset, ULONGEST len);

diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index 1e922b9..6cba0c2 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -195,6 +195,38 @@ linux_proc_pid_get_ns (pid_t pid, const char *ns)
   return NULL;
 }

+char *
+linux_proc_tid_get_name (ptid_t ptid)
+{
+  char buf[100];
+  char commbuf[64];
+  char* commval;
+  FILE *comm_file;
+
+  xsnprintf (buf, sizeof (buf), "/proc/%ld/task/%ld/comm",
+	     (long) ptid_get_pid (ptid),
+	     (long) (ptid_lwp_p (ptid)
+		     ? ptid_get_lwp (ptid)
+		     : ptid_get_pid (ptid)));
+
+  comm_file = gdb_fopen_cloexec (buf, "r");
+  if (comm_file == NULL)
+    return NULL;
+
+  commval = fgets (commbuf, sizeof (commbuf), comm_file);
+  fclose (comm_file);
+
+  if (commval)
+    {
+      if (commval[0] != '\0' && commval[strlen (commval) - 1] == '\n')
+	commval[strlen (commval) - 1] = '\0';
+      return xstrdup (commval);
+    }
+
+  return NULL;
+}
+
+
 /* See linux-procfs.h.  */

 void
diff --git a/gdb/nat/linux-procfs.h b/gdb/nat/linux-procfs.h
index 979ae0d..ef70aff 100644
--- a/gdb/nat/linux-procfs.h
+++ b/gdb/nat/linux-procfs.h
@@ -58,6 +58,11 @@ extern int linux_proc_pid_is_gone (pid_t pid);

 extern char *linux_proc_pid_get_ns (pid_t pid, const char *ns);

+/* Return an opaque string giving the thread's name or NULL if the
+   information is unavailable.  The returned string must be released
+   with xfree.  */
+extern char *linux_proc_tid_get_name (ptid_t ptid);
+
 /* Callback function for linux_proc_attach_tgid_threads.  If the PTID
    thread is not yet known, try to attach to it and return true,
    otherwise return false.  */
diff --git a/gdb/remote.c b/gdb/remote.c
index 8f783a4..1f3c8b6 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -382,6 +382,7 @@ struct remote_state
 struct private_thread_info
 {
   char *extra;
+  char *name;
   int core;
 };

@@ -389,6 +390,7 @@ static void
 free_private_thread_info (struct private_thread_info *info)
 {
   xfree (info->extra);
+  xfree (info->name);
   xfree (info);
 }

@@ -1915,6 +1917,17 @@ remote_thread_alive (struct target_ops *ops,
ptid_t ptid)
   return (rs->buf[0] == 'O' && rs->buf[1] == 'K');
 }

+/* Return a pointer to a thread name if we know it and NULL otherwise.
+   The thread_info object owns the memory for the name.  */
+static char*
+remote_thread_name (struct target_ops *ops, struct thread_info *info)
+{
+  if (info && info->priv)
+    return info->priv->name;
+
+  return NULL;
+}
+
 /* About these extended threadlist and threadinfo packets.  They are
    variable length packets but, the fields within them are often fixed
    length.  They are redundent enough to send over UDP as is the
@@ -2587,6 +2600,9 @@ typedef struct thread_item
   /* The thread's extra info.  May be NULL.  */
   char *extra;

+  /* The thread's name.  May be NULL.  */
+  char *name;
+
   /* The core the thread was running on.  -1 if not known.  */
   int core;
 } thread_item_t;
@@ -2612,7 +2628,10 @@ clear_threads_listing_context (void *p)
   struct thread_item *item;

   for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
-    xfree (item->extra);
+    {
+      xfree (item->extra);
+      xfree (item->name);
+    }

   VEC_free (thread_item_t, context->items);
 }
@@ -2683,6 +2702,9 @@ start_thread (struct gdb_xml_parser *parser,
   else
     item.core = -1;

+  attr = xml_find_attribute (attributes, "name");
+  item.name = attr ? xstrdup (attr->value) : NULL;
+
   item.extra = 0;

   VEC_safe_push (thread_item_t, data->items, &item);
@@ -2702,6 +2724,7 @@ end_thread (struct gdb_xml_parser *parser,
 const struct gdb_xml_attribute thread_attributes[] = {
   { "id", GDB_XML_AF_NONE, NULL, NULL },
   { "core", GDB_XML_AF_OPTIONAL, gdb_xml_parse_attr_ulongest, NULL },
+  { "name", GDB_XML_AF_OPTIONAL, NULL, NULL },
   { NULL, GDB_XML_AF_NONE, NULL, NULL }
 };

@@ -2875,6 +2898,8 @@ remote_update_thread_list (struct target_ops *ops)
 	      info->core = item->core;
 	      info->extra = item->extra;
 	      item->extra = NULL;
+	      info->name = item->name;
+	      item->name = NULL;
 	    }
 	}
     }
@@ -11729,6 +11754,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_pass_signals = remote_pass_signals;
   remote_ops.to_program_signals = remote_program_signals;
   remote_ops.to_thread_alive = remote_thread_alive;
+  remote_ops.to_thread_name = remote_thread_name;
   remote_ops.to_update_thread_list = remote_update_thread_list;
   remote_ops.to_pid_to_str = remote_pid_to_str;
   remote_ops.to_extra_thread_info = remote_threads_extra_info;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] display names of remote threads
  2015-05-14 22:26 [PATCH] display names of remote threads Daniel Colascione
@ 2015-05-15  7:27 ` Eli Zaretskii
  2015-05-19 11:42 ` Pedro Alves
  1 sibling, 0 replies; 3+ messages in thread
From: Eli Zaretskii @ 2015-05-15  7:27 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: gdb-patches

> Date: Thu, 14 May 2015 15:25:58 -0700
> From: Daniel Colascione <dancol@dancol.org>
> 
> commit 9b8ccc79e8a522b20ac6413751fa488622131839
> Author: Daniel Colascione <dancol@dancol.org>
> Date:   Thu May 14 15:24:08 2015 -0700
> 
>     Display names of remote threads

Please also include changes to corresponding ChangeLog files.

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 4b76ce9..341ed82 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -39111,17 +39111,19 @@ the following structure:
>  @smallexample
>  <?xml version="1.0"?>
>  <threads>
> -    <thread id="id" core="0">
> +    <thread id="id" core="0" name="name">
>      ... description ...
>      </thread>
>  </threads>
>  @end smallexample
> 
>  Each @samp{thread} element must have the @samp{id} attribute that
> -identifies the thread (@pxref{thread-id syntax}).  The
> -@samp{core} attribute, if present, specifies which processor core
> -the thread was last executing on.  The content of the of @samp{thread}
> -element is interpreted as human-readable auxilliary information.
> +identifies the thread (@pxref{thread-id syntax}).  The @samp{core}
> +attribute, if present, specifies which processor core the thread was
> +last executing on.  The @samp{name} attribute, if present, specifies
> +the human-readable name of the thread.  The content of the of
> +@samp{thread} element is interpreted as human-readable
> +auxilliary information.

Something is wrong with the last sentence.  (I actually wonder why
that sentence is needed: it's not clear to me what does it want to
convey.)

The documentation part is OK with this fixed.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] display names of remote threads
  2015-05-14 22:26 [PATCH] display names of remote threads Daniel Colascione
  2015-05-15  7:27 ` Eli Zaretskii
@ 2015-05-19 11:42 ` Pedro Alves
  1 sibling, 0 replies; 3+ messages in thread
From: Pedro Alves @ 2015-05-19 11:42 UTC (permalink / raw)
  To: Daniel Colascione, gdb-patches

Hi Daniel,

Thanks for doing this.

On 05/14/2015 11:25 PM, Daniel Colascione wrote:
> This patch adds an additional qXfer:threads:read attribute
> for thread names.
> 
> commit 9b8ccc79e8a522b20ac6413751fa488622131839
> Author: Daniel Colascione <dancol@dancol.org>
> Date:   Thu May 14 15:24:08 2015 -0700
> 
>     Display names of remote threads
> 

Please add the info above to the commit log.  Also please cross
check https://sourceware.org/gdb/wiki/ContributionChecklist:

 - This being a new RSP packet feature, should have a NEWS entry.
 - Before/after gdb output would be good to have.
 - Missing ChangeLog.

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 4b76ce9..341ed82 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -39111,17 +39111,19 @@ the following structure:
>  @smallexample
>  <?xml version="1.0"?>
>  <threads>
> -    <thread id="id" core="0">
> +    <thread id="id" core="0" name="name">
>      ... description ...
>      </thread>
>  </threads>
>  @end smallexample
> 
>  Each @samp{thread} element must have the @samp{id} attribute that
> -identifies the thread (@pxref{thread-id syntax}).  The
> -@samp{core} attribute, if present, specifies which processor core
> -the thread was last executing on.  The content of the of @samp{thread}
> -element is interpreted as human-readable auxilliary information.
> +identifies the thread (@pxref{thread-id syntax}).  The @samp{core}
> +attribute, if present, specifies which processor core the thread was
> +last executing on.  The @samp{name} attribute, if present, specifies
> +the human-readable name of the thread.  The content of the of
> +@samp{thread} element is interpreted as human-readable
> +auxilliary information.
> 
>  @node Traceframe Info Format
>  @section Traceframe Info Format
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index c6aa710..a72f862 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -6255,6 +6255,7 @@ static struct target_ops linux_target_ops = {
>    NULL,
>  #endif
>    linux_supports_range_stepping,
> +  linux_common_name_of_thread,
>  };
> 
>  static void
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index 83529ff..7dc99f1 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -1355,20 +1355,23 @@ handle_qxfer_threads_worker (struct
> inferior_list_entry *inf, void *arg)
>    char ptid_s[100];
>    int core = target_core_of_thread (ptid);
>    char core_s[21];
> +  char *name = NULL;
> 
>    write_ptid (ptid_s, ptid);
> 
> +  buffer_xml_printf (buffer, "<thread id=\"%s\"", ptid_s);
> +
>    if (core != -1)
>      {
>        sprintf (core_s, "%d", core);
> -      buffer_xml_printf (buffer, "<thread id=\"%s\" core=\"%s\"/>\n",
> -			 ptid_s, core_s);
> -    }
> -  else
> -    {
> -      buffer_xml_printf (buffer, "<thread id=\"%s\"/>\n",
> -			 ptid_s);
> +      buffer_xml_printf (buffer, " core=\"%s\"", core_s);
>      }
> +
> +  name = target_thread_name (ptid);
> +  if (name != NULL)
> +    buffer_xml_printf (buffer, " name=\"%s\"", name);
> +
> +  buffer_xml_printf (buffer, "/>\n");
>  }
> 
>  /* Helper for handle_qxfer_threads.  */
> diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
> index 126c861..edc547c 100644
> --- a/gdb/gdbserver/target.h
> +++ b/gdb/gdbserver/target.h
> @@ -394,6 +394,9 @@ struct target_ops
> 
>    /* Return true if target supports range stepping.  */
>    int (*supports_range_stepping) (void);
> +
> +  /* Return name of thread if known.  */
> +  char *(*thread_name) (ptid_t);
>  };
> 
>  extern struct target_ops *the_target;
> @@ -569,6 +572,10 @@ ptid_t mywait (ptid_t ptid, struct
> target_waitstatus *ourstatus, int options,
>    (the_target->core_of_thread ? (*the_target->core_of_thread) (ptid) \
>     : -1)
> 
> +#define target_thread_name(ptid)                                \
> +  (the_target->thread_name ? (*the_target->thread_name) (ptid)  \
> +   : NULL)
> +
>  int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int
> len);
> 
>  int write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 4a5a066..29fc340 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -3924,38 +3924,7 @@ linux_nat_pid_to_str (struct target_ops *ops,
> ptid_t ptid)
>  static char *
>  linux_nat_thread_name (struct target_ops *self, struct thread_info *thr)
>  {
> -  int pid = ptid_get_pid (thr->ptid);
> -  long lwp = ptid_get_lwp (thr->ptid);
> -#define FORMAT "/proc/%d/task/%ld/comm"
> -  char buf[sizeof (FORMAT) + 30];
> -  FILE *comm_file;
> -  char *result = NULL;
> -
> -  snprintf (buf, sizeof (buf), FORMAT, pid, lwp);
> -  comm_file = gdb_fopen_cloexec (buf, "r");
> -  if (comm_file)
> -    {
> -      /* Not exported by the kernel, so we define it here.  */
> -#define COMM_LEN 16
> -      static char line[COMM_LEN + 1];
> -
> -      if (fgets (line, sizeof (line), comm_file))
> -	{
> -	  char *nl = strchr (line, '\n');
> -
> -	  if (nl)
> -	    *nl = '\0';
> -	  if (*line != '\0')
> -	    result = line;
> -	}
> -
> -      fclose (comm_file);
> -    }
> -
> -#undef COMM_LEN
> -#undef FORMAT
> -
> -  return result;
> +  return linux_proc_tid_get_name (thr->ptid);
>  }
> 
>  /* Accepts an integer PID; Returns a string representing a file that
> diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
> index 0ed5d34..6cb01e6 100644
> --- a/gdb/nat/linux-osdata.c
> +++ b/gdb/nat/linux-osdata.c
> @@ -34,6 +34,7 @@
> 
>  #include "xml-utils.h"
>  #include "buffer.h"
> +#include "linux-procfs.h"
>  #include <dirent.h>
>  #include <sys/stat.h>
>  #include "filestuff.h"
> @@ -109,6 +110,22 @@ linux_common_core_of_thread (ptid_t ptid)
>    return core;
>  }
> 

I'd rather leave linux-osdata.c strictly for "info os foo" support,
but linux_common_core_of_thread is here already, so fine to
follow precedent.  That said, how about making linux_proc_tid_get_name
return a static buffer itself, and use that directly
instead of going through linux_common_name_of_thread?

Because ...

> @@ -3924,38 +3924,7 @@ linux_nat_pid_to_str (struct target_ops *ops,
> ptid_t ptid)
>  static char *
>  linux_nat_thread_name (struct target_ops *self, struct thread_info *thr)
>  {
> -  int pid = ptid_get_pid (thr->ptid);
> -  long lwp = ptid_get_lwp (thr->ptid);
> -#define FORMAT "/proc/%d/task/%ld/comm"
> -  char buf[sizeof (FORMAT) + 30];
> -  FILE *comm_file;
> -  char *result = NULL;
> -
> -  snprintf (buf, sizeof (buf), FORMAT, pid, lwp);
> -  comm_file = gdb_fopen_cloexec (buf, "r");
> -  if (comm_file)
> -    {
> -      /* Not exported by the kernel, so we define it here.  */
> -#define COMM_LEN 16
> -      static char line[COMM_LEN + 1];
> -
> -      if (fgets (line, sizeof (line), comm_file))
> -	{
> -	  char *nl = strchr (line, '\n');
> -
> -	  if (nl)
> -	    *nl = '\0';
> -	  if (*line != '\0')
> -	    result = line;
> -	}
> -
> -      fclose (comm_file);
> -    }
> -
> -#undef COMM_LEN
> -#undef FORMAT
> -
> -  return result;
> +  return linux_proc_tid_get_name (thr->ptid);
>  }

... it seems to me that before, this was returning a pointer to
a static buffer, but after it returns returns a pointer to an
allocated buffer, which ends up leaked by callers.


> +char *
> +linux_common_name_of_thread (ptid_t ptid)

Misses intro comment.  We prefer putting those in the .h file
nowadays, and add:

/* See linux-osdata.h.  */

in the .c file.

> +{
> +  static char buf[16 /*kernel maximum */ + 1];
> +  char* name = linux_proc_tid_get_name (ptid);

'char *name'

> +  if (name)

  if (name != NULL)


> +    {
> +      snprintf (buf, sizeof (buf), "%s", name);

xsnprintf.

> +      free (name);
> +    }
> +  else
> +    buf[0] = '\0';
> +
> +  return buf;
> +}
> +
>  /* Finds the command-line of process PID and copies it into COMMAND.
>     At most MAXLEN characters are copied.  If the command-line cannot
>     be found, PID is copied into command in text-form.  */
> diff --git a/gdb/nat/linux-osdata.h b/gdb/nat/linux-osdata.h
> index db8b445..1fdf367 100644
> --- a/gdb/nat/linux-osdata.h
> +++ b/gdb/nat/linux-osdata.h
> @@ -21,6 +21,7 @@
>  #define COMMON_LINUX_OSDATA_H
> 
>  extern int linux_common_core_of_thread (ptid_t ptid);
> +extern char *linux_common_name_of_thread (ptid_t ptid);
>  extern LONGEST linux_common_xfer_osdata (const char *annex, gdb_byte
> *readbuf,
>  					 ULONGEST offset, ULONGEST len);
> 
> diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
> index 1e922b9..6cba0c2 100644
> --- a/gdb/nat/linux-procfs.c
> +++ b/gdb/nat/linux-procfs.c
> @@ -195,6 +195,38 @@ linux_proc_pid_get_ns (pid_t pid, const char *ns)
>    return NULL;
>  }
> 
> +char *
> +linux_proc_tid_get_name (ptid_t ptid)

/* See foo.h.  */

> +{
> +  char buf[100];
> +  char commbuf[64];
> +  char* commval;

Space before, not after '*':

  char *commval;

> +  FILE *comm_file;
> +
> +  xsnprintf (buf, sizeof (buf), "/proc/%ld/task/%ld/comm",
> +	     (long) ptid_get_pid (ptid),
> +	     (long) (ptid_lwp_p (ptid)
> +		     ? ptid_get_lwp (ptid)
> +		     : ptid_get_pid (ptid)));
> +
> +  comm_file = gdb_fopen_cloexec (buf, "r");
> +  if (comm_file == NULL)
> +    return NULL;
> +
> +  commval = fgets (commbuf, sizeof (commbuf), comm_file);
> +  fclose (comm_file);
> +
> +  if (commval)

  if (commval != NULL)


> +    {
> +      if (commval[0] != '\0' && commval[strlen (commval) - 1] == '\n')
> +	commval[strlen (commval) - 1] = '\0';
> +      return xstrdup (commval);
> +    }
> +
> +  return NULL;
> +}
> +
> +
>  /* See linux-procfs.h.  */
> 
>  void
> diff --git a/gdb/nat/linux-procfs.h b/gdb/nat/linux-procfs.h
> index 979ae0d..ef70aff 100644
> --- a/gdb/nat/linux-procfs.h
> +++ b/gdb/nat/linux-procfs.h
> @@ -58,6 +58,11 @@ extern int linux_proc_pid_is_gone (pid_t pid);
> 
>  extern char *linux_proc_pid_get_ns (pid_t pid, const char *ns);
> 
> +/* Return an opaque string giving the thread's name or NULL if the
> +   information is unavailable.  The returned string must be released
> +   with xfree.  */
> +extern char *linux_proc_tid_get_name (ptid_t ptid);
> +
>  /* Callback function for linux_proc_attach_tgid_threads.  If the PTID
>     thread is not yet known, try to attach to it and return true,
>     otherwise return false.  */
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 8f783a4..1f3c8b6 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -382,6 +382,7 @@ struct remote_state
>  struct private_thread_info
>  {
>    char *extra;
> +  char *name;
>    int core;
>  };
> 
> @@ -389,6 +390,7 @@ static void
>  free_private_thread_info (struct private_thread_info *info)
>  {
>    xfree (info->extra);
> +  xfree (info->name);
>    xfree (info);
>  }
> 
> @@ -1915,6 +1917,17 @@ remote_thread_alive (struct target_ops *ops,
> ptid_t ptid)
>    return (rs->buf[0] == 'O' && rs->buf[1] == 'K');
>  }
> 
> +/* Return a pointer to a thread name if we know it and NULL otherwise.
> +   The thread_info object owns the memory for the name.  */
> +static char*

Add empty line between comment and function.  Space before '*'.

> +remote_thread_name (struct target_ops *ops, struct thread_info *info)
> +{
> +  if (info && info->priv)

  if (info != NULL && info->priv != NULL)

> +    return info->priv->name;
> +
> +  return NULL;
> +}
> +
>  /* About these extended threadlist and threadinfo packets.  They are
>     variable length packets but, the fields within them are often fixed
>     length.  They are redundent enough to send over UDP as is the
> @@ -2587,6 +2600,9 @@ typedef struct thread_item
>    /* The thread's extra info.  May be NULL.  */
>    char *extra;
> 
> +  /* The thread's name.  May be NULL.  */
> +  char *name;
> +
>    /* The core the thread was running on.  -1 if not known.  */
>    int core;
>  } thread_item_t;
> @@ -2612,7 +2628,10 @@ clear_threads_listing_context (void *p)
>    struct thread_item *item;
> 
>    for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
> -    xfree (item->extra);
> +    {
> +      xfree (item->extra);
> +      xfree (item->name);
> +    }
> 
>    VEC_free (thread_item_t, context->items);
>  }
> @@ -2683,6 +2702,9 @@ start_thread (struct gdb_xml_parser *parser,
>    else
>      item.core = -1;
> 
> +  attr = xml_find_attribute (attributes, "name");
> +  item.name = attr ? xstrdup (attr->value) : NULL;

attr != NULL.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-05-19 11:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-14 22:26 [PATCH] display names of remote threads Daniel Colascione
2015-05-15  7:27 ` Eli Zaretskii
2015-05-19 11:42 ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).