[PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Will Huang]

Test OS:Linux  2.6.32-220.23.2.ali1089.el5.x86_64 
GNU gdb (GDB) 7.6

Under linux kernel 2.6.32( or earlier version) there is no proc file named
/proc/$pid/task/$tid/comm. 
GDB init their threads' name from this proc file. So info threads can't
print correct thread names when 
This proc file not exist. This can be solve by using procfs
/proc/$pid/task/$tid/status instead if comm file 
not exists. Status file has a Name line which starts by "Name:".

diff -ruN ./gdb.org/ChangeLog ./gdb/ChangeLog
--- ./gdb.org/ChangeLog 2013-04-26 19:43:30.000000000 +0800
+++ ./gdb/ChangeLog     2013-08-14 11:09:08.000000000 +0800
@@ -1,3 +1,11 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * linux-nat.c (linux_nat_thread_name): Add get thread name from
+       procfs "/proc/$pid/task/$tid/status".
+       * commo/linux-procfs.c (linux_proc_get_thread_name): New.
+       * commo/linux-procfs.c (linux_proc_get_string): New.
+
 2013-04-26  Joel Brobecker  <brobecker@adacore.com>

        * NEWS: Change "since GDB 7.5" into "in GDB 7.6".
diff -ruN ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
--- ./gdb.org/common/linux-procfs.c     2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.c 2013-08-12 16:51:33.000000000 +0800
@@ -55,6 +55,51 @@
   return retval;
 }

+/* Return the string length of Name from /proc/$pid/task/$tid/status.
+   Returns -1 if not found.  */
+
+static int
+linux_proc_get_string (pid_t lwpid, pid_t tid,
+                       char *target, size_t t_size, const char *field)
+{
+  size_t field_len = strlen (field);
+  FILE *status_file;
+  char buf[100];
+  int retval = -1;
+
+  if (tid > 0)
+       snprintf (buf, sizeof (buf),
+                  "/proc/%d/task/%d/status", (int) lwpid, (int) tid);
+  else
+       snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
+
+  status_file = fopen (buf, "r");
+  if (status_file == NULL)
+    {
+      warning (_("unable to open /proc file '%s'"), buf);
+      return -1;
+    }
+
+  while (fgets (buf, sizeof (buf), status_file))
+    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
+      {
+       size_t pos = field_len + 1;
+       while((buf[pos] == ' ' || buf[pos] == '\t') && (pos < sizeof (buf) -
1))
+         pos++;
+
+       strncpy (target, &buf[pos], t_size);
+       target[t_size - 1] = '\0';
+       retval = strlen (target);
+       if(retval > 0 && target[retval - 1] == '\n')
+               target[retval - 1] = '\0';
+
+       break;
+      }
+
+  fclose (status_file);
+  return retval;
+}
+
 /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
    found.  */

@@ -118,3 +163,11 @@
 {
   return linux_proc_pid_has_state (pid, "Z (zombie)");
 }
+
+/*Get thread name identified by PID & TID*/
+
+int
+linux_proc_get_thread_name (pid_t pid, pid_t tid, char *name, size_t size)
+{
+  return linux_proc_get_string (pid, tid, name, size, "Name");
+}
diff -ruN ./gdb.org/common/linux-procfs.h ./gdb/common/linux-procfs.h
--- ./gdb.org/common/linux-procfs.h     2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.h 2013-08-12 16:50:39.000000000 +0800
@@ -40,4 +40,10 @@

 extern int linux_proc_pid_is_zombie (pid_t pid);

+/* Return -1 if thread Name of tid didn't found;
+   "char *name" for thread name (need be allocated by caller).  */
+
+extern int linux_proc_get_thread_name (pid_t pid, pid_t tid,
+                                       char *name, size_t size);
+
 #endif /* COMMON_LINUX_PROCFS_H */
diff -ruN ./gdb.org/linux-nat.c ./gdb/linux-nat.c
--- ./gdb.org/linux-nat.c       2013-02-13 22:59:49.000000000 +0800
+++ ./gdb/linux-nat.c   2013-08-12 16:47:26.000000000 +0800
@@ -4294,6 +4294,14 @@

       fclose (comm_file);
     }
+    else
+    {
+      static char comm[COMM_LEN + 1];
+      int size = linux_proc_get_thread_name (pid, lwp, comm, COMM_LEN+1);
+      if (size > 0)
+        result = comm;
+
+    }

 #undef COMM_LEN
 #undef FORMAT
diff -ruN ./gdb.org/testsuite/ChangeLog ./gdb/testsuite/ChangeLog
--- ./gdb.org/testsuite/ChangeLog       2013-04-25 20:22:26.000000000 +0800
+++ ./gdb/testsuite/ChangeLog   2013-08-14 12:18:23.000000000 +0800
@@ -1,3 +1,9 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * gdb.threads/manythreads.exp: Add test for get thread
+       original name
+
 2013-04-25  Sergio Durigan Junior  <sergiodj@redhat.com>

        * gdb.arch/arm-bl-branch-dest.c: New file.
diff -ruN ./gdb.org/testsuite/gdb.threads/manythreads.exp
./gdb/testsuite/gdb.threads/manythreads.exp
--- ./gdb.org/testsuite/gdb.threads/manythreads.exp     2013-01-01
14:41:27.000000000 +0800
+++ ./gdb/testsuite/gdb.threads/manythreads.exp 2013-08-14
12:15:07.000000000 +0800
@@ -93,6 +93,7 @@
     }
 }

+gdb_test "info threads" ".*manythreads.*" "check thread original name"
 gdb_test_no_output "thread name zardoz" "give a name to the thread"
 gdb_test "info threads" ".*zardoz.*" "check thread name"



Thanks,
Will Huang

Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Yao Qi]

Hi Will,
I am not the 'right' person to review this patch, however, I find some
thing obviously incorrect.  I point them out here.

On 08/14/2013 12:26 PM, Will Huang wrote:
> diff -ruN ./gdb.org/ChangeLog ./gdb/ChangeLog
> --- ./gdb.org/ChangeLog 2013-04-26 19:43:30.000000000 +0800
> +++ ./gdb/ChangeLog     2013-08-14 11:09:08.000000000 +0800
> @@ -1,3 +1,11 @@
> +2013-08-14  Will Huang<will.huang@aliyun-inc.com>
> +
> +       PR threads/15824

Each line should be prefixed with 'TAB' instead of 8 spaces.

> +       * linux-nat.c (linux_nat_thread_name): Add get thread name from
> +       procfs "/proc/$pid/task/$tid/status".
> +       * commo/linux-procfs.c (linux_proc_get_thread_name): New.
> +       * commo/linux-procfs.c (linux_proc_get_string): New.
           ^^^^^^^^^^^^^^^^^^^^^^^  Duplicated.

Only have to mention the same file once.

> +
>   2013-04-26  Joel Brobecker<brobecker@adacore.com>
>
>          * NEWS: Change "since GDB 7.5" into "in GDB 7.6".
> diff -ruN ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
> --- ./gdb.org/common/linux-procfs.c     2013-01-01 14:32:54.000000000 +0800
> +++ ./gdb/common/linux-procfs.c 2013-08-12 16:51:33.000000000 +0800
> @@ -55,6 +55,51 @@
>     return retval;
>   }
>
> +/* Return the string length of Name from/proc/$pid/task/$tid/status.

Space after "from" is needed.
s/Name/FIELD
s/$pid/LWPID
s/$tid/TID

> +   Returns -1 if not found.  */

s/Returns/Return

> +
> +static int
> +linux_proc_get_string (pid_t lwpid, pid_t tid,
> +                       char *target, size_t t_size, const char *field)

"char *target" should go to the previous line.

> +{
> +  size_t field_len = strlen (field);
> +  FILE *status_file;
> +  char buf[100];
> +  int retval = -1;
> +
> +  if (tid > 0)
> +       snprintf (buf, sizeof (buf),
> +                  "/proc/%d/task/%d/status", (int) lwpid, (int) tid);

Use xsnprintf, and the indentation looks wrong.

> +  else
> +       snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
> +

Likewise.  Is it a dead path?  Can 'tid' be negative?

> +  status_file = fopen (buf, "r");

Use gdb_fopen_cloexec?

> +  if (status_file == NULL)
> +    {
> +      warning (_("unable to open /proc file '%s'"), buf);
> +      return -1;
> +    }
> +
> +  while (fgets (buf, sizeof (buf), status_file))
> +    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
> +      {
> +       size_t pos = field_len + 1;
> +       while((buf[pos] == ' ' || buf[pos] == '\t') && (pos < sizeof (buf) -
> 1))

Your mailer wrap this line...

> +         pos++;
> +
> +       strncpy (target, &buf[pos], t_size);
> +       target[t_size - 1] = '\0';
> +       retval = strlen (target);
> +       if(retval > 0 && target[retval - 1] == '\n')
> +               target[retval - 1] = '\0';
> +
> +       break;
> +      }
> +
> +  fclose (status_file);
> +  return retval;
> +}
> +
>   /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
>      found.  */
>
> @@ -118,3 +163,11 @@
>   {
>     return linux_proc_pid_has_state (pid, "Z (zombie)");
>   }
> +
> +/*Get thread name identified by PID & TID*/

I am not good at comments either, but I suggest

/* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
    of buffer NAME.  Return the length of the name if success,
    otherwise return -1.  */

> +
> +int
> +linux_proc_get_thread_name (pid_t pid, pid_t tid, char *name, size_t size)
> +{
> +  return linux_proc_get_string (pid, tid, name, size, "Name");
> +}
> diff -ruN ./gdb.org/common/linux-procfs.h ./gdb/common/linux-procfs.h
> --- ./gdb.org/common/linux-procfs.h     2013-01-01 14:32:54.000000000 +0800
> +++ ./gdb/common/linux-procfs.h 2013-08-12 16:50:39.000000000 +0800
> @@ -40,4 +40,10 @@
>
>   extern int linux_proc_pid_is_zombie (pid_t pid);
>
> +/* Return -1 if thread Name of tid didn't found;

Comments should be updated as what I suggested above...

> +   "char *name" for thread name (need be allocated by caller).  */
> +
> +extern int linux_proc_get_thread_name (pid_t pid, pid_t tid,
> +                                       char *name, size_t size);
> +
>   #endif /* COMMON_LINUX_PROCFS_H */
> diff -ruN ./gdb.org/linux-nat.c ./gdb/linux-nat.c
> --- ./gdb.org/linux-nat.c       2013-02-13 22:59:49.000000000 +0800
> +++ ./gdb/linux-nat.c   2013-08-12 16:47:26.000000000 +0800
> @@ -4294,6 +4294,14 @@
>
>         fclose (comm_file);
>       }
> +    else
> +    {
> +      static char comm[COMM_LEN + 1];
> +      int size = linux_proc_get_thread_name (pid, lwp, comm, COMM_LEN+1);

A blank line is needed.  Space is needed before and after "+".

The type of pid is int and type of lwp is long, but the types of 
parameters of linux_proc_get_thread_name are pid_t and pid_t.  It is 
inconsistent.

-- 
Yao (齐尧)

RE: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Will Huang]

Thanks for Yao Qi's review.
Patch is update.

Test OS:Linux  2.6.32-220.23.2.ali1089.el5.x86_64
GNU gdb (GDB) 7.6

Under linux kernel 2.6.32( or earlier version) there is no proc file named /proc/$pid/task/$tid/comm. 
GDB init their threads' name from this proc file. So info threads can't print correct thread names when
this proc file not exist. This can be solve by using procfs /proc/$pid/task/$tid/status instead if comm 
file not exists. Status file has a Name line which starts by "Name:".


diff -uNr ./gdb.org/ChangeLog ./gdb/ChangeLog
--- ./gdb.org/ChangeLog 2013-04-26 19:43:30.000000000 +0800
+++ ./gdb/ChangeLog     2013-08-16 11:48:20.000000000 +0800
@@ -1,3 +1,11 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * linux-nat.c (linux_nat_thread_name): Add get thread name from
+       procfs "/proc/$pid/task/$tid/status".
+       * commo/linux-procfs.c (linux_proc_get_thread_name
+       linux_proc_get_string): New.
+
 2013-04-26  Joel Brobecker  <brobecker@adacore.com>

        * NEWS: Change "since GDB 7.5" into "in GDB 7.6".
diff -uNr ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
--- ./gdb.org/common/linux-procfs.c     2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.c 2013-08-16 11:48:49.000000000 +0800
@@ -55,6 +55,55 @@
   return retval;
 }

+/* Return the string length of FILED /proc/LWPID/task/TID/status.
+   Return -1 if not found.  */
+
+static int
+linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
+                       size_t t_size, const char *field)
+{
+  size_t field_len = strlen (field);
+  FILE *status_file;
+  char buf[100];
+  int retval = -1;
+
+  if (tid > 0)
+       snprintf (buf, sizeof (buf), "/proc/%d/task/%d/status",
+                                       (int) lwpid, (int) tid);
+  else
+  {
+       /*if TID is zero, ingnore it*/
+       snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
+  }
+
+  status_file = fopen (buf, "r");
+  if (status_file == NULL)
+    {
+      warning (_("unable to open /proc file '%s'"), buf);
+      return -1;
+    }
+
+  while (fgets (buf, sizeof (buf), status_file))
+    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
+      {
+       size_t pos = field_len + 1;
+       while((buf[pos] == ' ' || buf[pos] == '\t')
+                       && (pos < sizeof (buf) - 1))
+         pos++;
+
+       strncpy (target, &buf[pos], t_size);
+       target[t_size - 1] = '\0';
+       retval = strlen (target);
+       if(retval > 0 && target[retval - 1] == '\n')
+               target[retval - 1] = '\0';
+
+       break;
+      }
+
+  fclose (status_file);
+  return retval;
+}
+
 /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
    found.  */

@@ -118,3 +167,13 @@
 {
   return linux_proc_pid_has_state (pid, "Z (zombie)");
 }
+
+/* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
+   of buffer NAME.  Return the length of the name if success,
+   otherwise return -1.  */
+
+int
+linux_proc_get_thread_name (pid_t pid, pid_t tid, char *name, size_t size)
+{
+  return linux_proc_get_string (pid, tid, name, size, "Name");
+}
diff -uNr ./gdb.org/common/linux-procfs.h ./gdb/common/linux-procfs.h
--- ./gdb.org/common/linux-procfs.h     2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.h 2013-08-16 11:48:49.000000000 +0800
@@ -40,4 +40,12 @@

 extern int linux_proc_pid_is_zombie (pid_t pid);

+
+/* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
+   of buffer NAME.  Return the length of the name if success,
+   otherwise return -1.  */
+
+extern int linux_proc_get_thread_name (pid_t pid, pid_t tid,
+                                       char *name, size_t size);
+
 #endif /* COMMON_LINUX_PROCFS_H */
diff -uNr ./gdb.org/linux-nat.c ./gdb/linux-nat.c
--- ./gdb.org/linux-nat.c       2013-02-13 22:59:49.000000000 +0800
+++ ./gdb/linux-nat.c   2013-08-16 13:22:31.000000000 +0800
@@ -4294,6 +4294,14 @@

       fclose (comm_file);
     }
+  else
+    {
+      static char comm[COMM_LEN + 1];
+      int size = linux_proc_get_thread_name (pid, lwp, comm, COMM_LEN + 1);
+      if (size > 0)
+        result = comm;
+
+    }

 #undef COMM_LEN
 #undef FORMAT
diff -uNr ./gdb.org/testsuite/ChangeLog ./gdb/testsuite/ChangeLog
--- ./gdb.org/testsuite/ChangeLog       2013-04-25 20:22:26.000000000 +0800
+++ ./gdb/testsuite/ChangeLog   2013-08-16 11:49:02.000000000 +0800
@@ -1,3 +1,9 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * gdb.threads/manythreads.exp: Add test for get thread
+       original name
+
 2013-04-25  Sergio Durigan Junior  <sergiodj@redhat.com>

        * gdb.arch/arm-bl-branch-dest.c: New file.
diff -uNr ./gdb.org/testsuite/gdb.threads/manythreads.exp ./gdb/testsuite/gdb.threads/manythreads.exp
--- ./gdb.org/testsuite/gdb.threads/manythreads.exp     2013-01-01 14:41:27.000000000 +0800
+++ ./gdb/testsuite/gdb.threads/manythreads.exp 2013-08-16 11:49:26.000000000 +0800
@@ -93,6 +93,7 @@
     }
 }

+gdb_test "info threads" ".*manythreads.*" "check thread original name"
 gdb_test_no_output "thread name zardoz" "give a name to the thread"
 gdb_test "info threads" ".*zardoz.*" "check thread name"



Thanks,
Will Huang


> -----Original Message-----
> From: Yao Qi [mailto:yao@codesourcery.com]
> Sent: Friday, August 16, 2013 10:26 AM
> To: Will Huang
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH][PR threads/15824] when get threads name failed from info
> threads with linux kernel version earlier than 2.6.33
> 
> Hi Will,
> I am not the 'right' person to review this patch, however, I find some thing
> obviously incorrect.  I point them out here.
> 
> On 08/14/2013 12:26 PM, Will Huang wrote:
> > diff -ruN ./gdb.org/ChangeLog ./gdb/ChangeLog
> > --- ./gdb.org/ChangeLog 2013-04-26 19:43:30.000000000 +0800
> > +++ ./gdb/ChangeLog     2013-08-14 11:09:08.000000000 +0800
> > @@ -1,3 +1,11 @@
> > +2013-08-14  Will Huang<will.huang@aliyun-inc.com>
> > +
> > +       PR threads/15824
> 
> Each line should be prefixed with 'TAB' instead of 8 spaces.
> 
> > +       * linux-nat.c (linux_nat_thread_name): Add get thread name from
> > +       procfs "/proc/$pid/task/$tid/status".
> > +       * commo/linux-procfs.c (linux_proc_get_thread_name): New.
> > +       * commo/linux-procfs.c (linux_proc_get_string): New.
>            ^^^^^^^^^^^^^^^^^^^^^^^  Duplicated.
> 
> Only have to mention the same file once.
> 
> > +
> >   2013-04-26  Joel Brobecker<brobecker@adacore.com>
> >
> >          * NEWS: Change "since GDB 7.5" into "in GDB 7.6".
> > diff -ruN ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
> > --- ./gdb.org/common/linux-procfs.c     2013-01-01 14:32:54.000000000 +0800
> > +++ ./gdb/common/linux-procfs.c 2013-08-12 16:51:33.000000000 +0800
> > @@ -55,6 +55,51 @@
> >     return retval;
> >   }
> >
> > +/* Return the string length of Name from/proc/$pid/task/$tid/status.
> 
> Space after "from" is needed.
> s/Name/FIELD
> s/$pid/LWPID
> s/$tid/TID
> 
> > +   Returns -1 if not found.  */
> 
> s/Returns/Return
> 
> > +
> > +static int
> > +linux_proc_get_string (pid_t lwpid, pid_t tid,
> > +                       char *target, size_t t_size, const char
> > +*field)
> 
> "char *target" should go to the previous line.
> 
> > +{
> > +  size_t field_len = strlen (field);
> > +  FILE *status_file;
> > +  char buf[100];
> > +  int retval = -1;
> > +
> > +  if (tid > 0)
> > +       snprintf (buf, sizeof (buf),
> > +                  "/proc/%d/task/%d/status", (int) lwpid, (int) tid);
> 
> Use xsnprintf, and the indentation looks wrong.
> 
> > +  else
> > +       snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
> > +
> 
> Likewise.  Is it a dead path?  Can 'tid' be negative?
> 
> > +  status_file = fopen (buf, "r");
> 
> Use gdb_fopen_cloexec?
> 
> > +  if (status_file == NULL)
> > +    {
> > +      warning (_("unable to open /proc file '%s'"), buf);
> > +      return -1;
> > +    }
> > +
> > +  while (fgets (buf, sizeof (buf), status_file))
> > +    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
> > +      {
> > +       size_t pos = field_len + 1;
> > +       while((buf[pos] == ' ' || buf[pos] == '\t') && (pos < sizeof
> > + (buf) -
> > 1))
> 
> Your mailer wrap this line...
> 
> > +         pos++;
> > +
> > +       strncpy (target, &buf[pos], t_size);
> > +       target[t_size - 1] = '\0';
> > +       retval = strlen (target);
> > +       if(retval > 0 && target[retval - 1] == '\n')
> > +               target[retval - 1] = '\0';
> > +
> > +       break;
> > +      }
> > +
> > +  fclose (status_file);
> > +  return retval;
> > +}
> > +
> >   /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
> >      found.  */
> >
> > @@ -118,3 +163,11 @@
> >   {
> >     return linux_proc_pid_has_state (pid, "Z (zombie)");
> >   }
> > +
> > +/*Get thread name identified by PID & TID*/
> 
> I am not good at comments either, but I suggest
> 
> /* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
>     of buffer NAME.  Return the length of the name if success,
>     otherwise return -1.  */
> 
> > +
> > +int
> > +linux_proc_get_thread_name (pid_t pid, pid_t tid, char *name, size_t
> > +size) {
> > +  return linux_proc_get_string (pid, tid, name, size, "Name"); }
> > diff -ruN ./gdb.org/common/linux-procfs.h ./gdb/common/linux-procfs.h
> > --- ./gdb.org/common/linux-procfs.h     2013-01-01 14:32:54.000000000
> +0800
> > +++ ./gdb/common/linux-procfs.h 2013-08-12 16:50:39.000000000 +0800
> > @@ -40,4 +40,10 @@
> >
> >   extern int linux_proc_pid_is_zombie (pid_t pid);
> >
> > +/* Return -1 if thread Name of tid didn't found;
> 
> Comments should be updated as what I suggested above...
> 
> > +   "char *name" for thread name (need be allocated by caller).  */
> > +
> > +extern int linux_proc_get_thread_name (pid_t pid, pid_t tid,
> > +                                       char *name, size_t size);
> > +
> >   #endif /* COMMON_LINUX_PROCFS_H */
> > diff -ruN ./gdb.org/linux-nat.c ./gdb/linux-nat.c
> > --- ./gdb.org/linux-nat.c       2013-02-13 22:59:49.000000000 +0800
> > +++ ./gdb/linux-nat.c   2013-08-12 16:47:26.000000000 +0800
> > @@ -4294,6 +4294,14 @@
> >
> >         fclose (comm_file);
> >       }
> > +    else
> > +    {
> > +      static char comm[COMM_LEN + 1];
> > +      int size = linux_proc_get_thread_name (pid, lwp, comm,
> > + COMM_LEN+1);
> 
> A blank line is needed.  Space is needed before and after "+".
> 
> The type of pid is int and type of lwp is long, but the types of parameters of
> linux_proc_get_thread_name are pid_t and pid_t.  It is inconsistent.
> 
> --
> Yao (齐尧)

Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Yao Qi]

Will,
not every comments are addressed by this updated patch.  I'll point them 
out again.  Please make sure your new patch will address all of them.

On 08/16/2013 01:32 PM, Will Huang wrote:
> +2013-08-14  Will Huang<will.huang@aliyun-inc.com>
> +
> +       PR threads/15824
    ^^^^^^^
Each line should be prefixed with 'TAB' instead of 8 spaces.

> +       * linux-nat.c (linux_nat_thread_name): Add get thread name from
> +       procfs "/proc/$pid/task/$tid/status".
> +       * commo/linux-procfs.c (linux_proc_get_thread_name
> +       linux_proc_get_string): New.

People prefer to give one entry for one function separately, like:

	* commo/linux-procfs.c (linux_proc_get_thread_name): New
	function.
	(linux_proc_get_string): New function.
> +
>   2013-04-26  Joel Brobecker<brobecker@adacore.com>
>
>          * NEWS: Change "since GDB 7.5" into "in GDB 7.6".
> diff -uNr ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
> --- ./gdb.org/common/linux-procfs.c     2013-01-01 14:32:54.000000000 +0800
> +++ ./gdb/common/linux-procfs.c 2013-08-16 11:48:49.000000000 +0800
> @@ -55,6 +55,55 @@
>     return retval;
>   }
>
> +/* Return the string length of FILED /proc/LWPID/task/TID/status.
> +   Return -1 if not found.  */
> +
> +static int
> +linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
> +                       size_t t_size, const char *field)
> +{
> +  size_t field_len = strlen (field);
> +  FILE *status_file;
> +  char buf[100];
> +  int retval = -1;
> +
> +  if (tid > 0)
> +       snprintf (buf, sizeof (buf), "/proc/%d/task/%d/status",
> +                                       (int) lwpid, (int) tid);

Use xsnprintf, and the indentation looks wrong.

> +  else
> +  {
> +       /*if TID is zero, ingnore it*/
> +       snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
> +  }

and here....

> +
> +  status_file = fopen (buf, "r");

Use gdb_fopen_cloexec?

> +  if (status_file == NULL)
> +    {
> +      warning (_("unable to open /proc file '%s'"), buf);
> +      return -1;
> +    }
> +
> +  while (fgets (buf, sizeof (buf), status_file))
> +    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
> +      {
> +       size_t pos = field_len + 1;
> +       while((buf[pos] == ' ' || buf[pos] == '\t')
> +                       && (pos < sizeof (buf) - 1))

Wrong indentation.

> +         pos++;
> +
> +       strncpy (target, &buf[pos], t_size);
> +       target[t_size - 1] = '\0';
> +       retval = strlen (target);
> +       if(retval > 0 && target[retval - 1] == '\n')
> +               target[retval - 1] = '\0';

Wrong indentation.


> diff -uNr ./gdb.org/linux-nat.c ./gdb/linux-nat.c
> --- ./gdb.org/linux-nat.c       2013-02-13 22:59:49.000000000 +0800
> +++ ./gdb/linux-nat.c   2013-08-16 13:22:31.000000000 +0800
> @@ -4294,6 +4294,14 @@
>
>         fclose (comm_file);
>       }
> +  else
> +    {
> +      static char comm[COMM_LEN + 1];
> +      int size = linux_proc_get_thread_name (pid, lwp, comm, COMM_LEN + 1);

A blank line is needed.

The type of pid is int and type of lwp is long, but the types of
parameters of linux_proc_get_thread_name are pid_t and pid_t.  It is
inconsistent.

> +      if (size > 0)
> +        result = comm;
> +
> +    }
>
>   #undef COMM_LEN
>   #undef FORMAT
> diff -uNr ./gdb.org/testsuite/ChangeLog ./gdb/testsuite/ChangeLog
> --- ./gdb.org/testsuite/ChangeLog       2013-04-25 20:22:26.000000000 +0800
> +++ ./gdb/testsuite/ChangeLog   2013-08-16 11:49:02.000000000 +0800
> @@ -1,3 +1,9 @@
> +2013-08-14  Will Huang<will.huang@aliyun-inc.com>
> +
> +       PR threads/15824
> +       * gdb.threads/manythreads.exp: Add test for get thread
> +       original name

Period is missing.

-- 
Yao (齐尧)

RE: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Will Huang]

Hello Qi,
  Sorry for mistakes. There is no gdb_fopen_cloexec in gdb-7.6.  Except that all comments are accepted.

Test OS:Linux  2.6.32-220.23.2.ali1089.el5.x86_64
GNU gdb (GDB) 7.6

Under linux kernel 2.6.32( or earlier version) there is no proc file named /proc/$pid/task/$tid/comm. 
GDB init their threads' name from this proc file. So info threads can't print correct thread names when 
This proc file not exist. This can be solve by using procfs /proc/$pid/task/$tid/status instead if comm file
 not exists. Status file has a Name line which starts by "Name:".

diff -ruN ./gdb.org/ChangeLog ./gdb/ChangeLog
--- ./gdb.org/ChangeLog 2013-04-26 19:43:30.000000000 +0800
+++ ./gdb/ChangeLog     2013-08-16 17:47:39.000000000 +0800
@@ -1,3 +1,11 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * linux-nat.c (linux_nat_thread_name): Add get thread name from
+       procfs "/proc/$pid/task/$tid/status".
+       * commo/linux-procfs.c (linux_proc_get_thread_name): New.
+       (linux_proc_get_string): New.
+
 2013-04-26  Joel Brobecker  <brobecker@adacore.com>

        * NEWS: Change "since GDB 7.5" into "in GDB 7.6".
diff -ruN ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
--- ./gdb.org/common/linux-procfs.c     2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.c 2013-08-16 17:51:44.000000000 +0800
@@ -55,6 +55,55 @@
   return retval;
 }

+/* Return the string length of FILED /proc/LWPID/task/TID/status.
+   Return -1 if not found.  */
+
+static int
+linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
+                       size_t t_size, const char *field)
+{
+  size_t field_len = strlen (field);
+  FILE *status_file;
+  char buf[100];
+  int retval = -1;
+
+  if (tid > 0)
+    xsnprintf (buf, sizeof (buf), "/proc/%d/task/%d/status",
+               (int) lwpid, (int) tid);
+  else
+  {
+     /*if TID is zero, ingnore it*/
+     xsnprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
+  }
+
+  status_file = fopen (buf, "r");
+  if (status_file == NULL)
+    {
+      warning (_("unable to open /proc file '%s'"), buf);
+      return -1;
+    }
+
+  while (fgets (buf, sizeof (buf), status_file))
+    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
+      {
+       size_t pos = field_len + 1;
+       while((buf[pos] == ' ' || buf[pos] == '\t')
+             && (pos < sizeof (buf) - 1))
+         pos++;
+
+       strncpy (target, &buf[pos], t_size);
+       target[t_size - 1] = '\0';
+       retval = strlen (target);
+       if (retval > 0 && target[retval - 1] == '\n')
+         target[retval - 1] = '\0';
+
+       break;
+      }
+
+  fclose (status_file);
+  return retval;
+}
+
 /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
    found.  */

@@ -118,3 +167,13 @@
 {
   return linux_proc_pid_has_state (pid, "Z (zombie)");
 }
+
+/* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
+   of buffer NAME.  Return the length of the name if success,
+   otherwise return -1.  */
+
+int
+linux_proc_get_thread_name (pid_t pid, pid_t tid, char *name, size_t size)
+{
+  return linux_proc_get_string (pid, tid, name, size, "Name");
+}
diff -ruN ./gdb.org/common/linux-procfs.h ./gdb/common/linux-procfs.h
--- ./gdb.org/common/linux-procfs.h     2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.h 2013-08-16 11:48:49.000000000 +0800
@@ -40,4 +40,12 @@

 extern int linux_proc_pid_is_zombie (pid_t pid);

+
+/* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
+   of buffer NAME.  Return the length of the name if success,
+   otherwise return -1.  */
+
+extern int linux_proc_get_thread_name (pid_t pid, pid_t tid,
+                                       char *name, size_t size);
+
 #endif /* COMMON_LINUX_PROCFS_H */
diff -ruN ./gdb.org/linux-nat.c ./gdb/linux-nat.c
--- ./gdb.org/linux-nat.c       2013-02-13 22:59:49.000000000 +0800
+++ ./gdb/linux-nat.c   2013-08-16 17:55:19.000000000 +0800
@@ -4294,6 +4294,15 @@

       fclose (comm_file);
     }
+  else
+    {
+      static char comm[COMM_LEN + 1];
+      int size = linux_proc_get_thread_name ((pid_t) pid, (pid_t) lwp, comm, COMM_LEN + 1);
+
+      if (size > 0)
+        result = comm;
+
+    }

 #undef COMM_LEN
 #undef FORMAT
diff -ruN ./gdb.org/testsuite/ChangeLog ./gdb/testsuite/ChangeLog
--- ./gdb.org/testsuite/ChangeLog       2013-04-25 20:22:26.000000000 +0800
+++ ./gdb/testsuite/ChangeLog   2013-08-16 17:55:49.000000000 +0800
@@ -1,3 +1,9 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * gdb.threads/manythreads.exp: Add test for get thread
+       original name.
+
 2013-04-25  Sergio Durigan Junior  <sergiodj@redhat.com>

        * gdb.arch/arm-bl-branch-dest.c: New file.
diff -ruN ./gdb.org/testsuite/gdb.threads/manythreads.exp ./gdb/testsuite/gdb.threads/manythreads.exp
--- ./gdb.org/testsuite/gdb.threads/manythreads.exp     2013-01-01 14:41:27.000000000 +0800
+++ ./gdb/testsuite/gdb.threads/manythreads.exp 2013-08-16 11:49:26.000000000 +0800
@@ -93,6 +93,7 @@
     }
 }

+gdb_test "info threads" ".*manythreads.*" "check thread original name"
 gdb_test_no_output "thread name zardoz" "give a name to the thread"
 gdb_test "info threads" ".*zardoz.*" "check thread name"




Thanks,
Will Huang


> -----Original Message-----
> From: Yao Qi [mailto:yao@codesourcery.com]
> Sent: Friday, August 16, 2013 5:38 PM
> To: Will Huang
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH][PR threads/15824] when get threads name failed from info
> threads with linux kernel version earlier than 2.6.33
> 
> Will,
> not every comments are addressed by this updated patch.  I'll point them out
> again.  Please make sure your new patch will address all of them.
> 
> On 08/16/2013 01:32 PM, Will Huang wrote:
> > +2013-08-14  Will Huang<will.huang@aliyun-inc.com>
> > +
> > +       PR threads/15824
>     ^^^^^^^
> Each line should be prefixed with 'TAB' instead of 8 spaces.
> 
> > +       * linux-nat.c (linux_nat_thread_name): Add get thread name from
> > +       procfs "/proc/$pid/task/$tid/status".
> > +       * commo/linux-procfs.c (linux_proc_get_thread_name
> > +       linux_proc_get_string): New.
> 
> People prefer to give one entry for one function separately, like:
> 
> 	* commo/linux-procfs.c (linux_proc_get_thread_name): New
> 	function.
> 	(linux_proc_get_string): New function.
> > +
> >   2013-04-26  Joel Brobecker<brobecker@adacore.com>
> >
> >          * NEWS: Change "since GDB 7.5" into "in GDB 7.6".
> > diff -uNr ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
> > --- ./gdb.org/common/linux-procfs.c     2013-01-01 14:32:54.000000000 +0800
> > +++ ./gdb/common/linux-procfs.c 2013-08-16 11:48:49.000000000 +0800
> > @@ -55,6 +55,55 @@
> >     return retval;
> >   }
> >
> > +/* Return the string length of FILED /proc/LWPID/task/TID/status.
> > +   Return -1 if not found.  */
> > +
> > +static int
> > +linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
> > +                       size_t t_size, const char *field) {
> > +  size_t field_len = strlen (field);
> > +  FILE *status_file;
> > +  char buf[100];
> > +  int retval = -1;
> > +
> > +  if (tid > 0)
> > +       snprintf (buf, sizeof (buf), "/proc/%d/task/%d/status",
> > +                                       (int) lwpid, (int) tid);
> 
> Use xsnprintf, and the indentation looks wrong.
> 
> > +  else
> > +  {
> > +       /*if TID is zero, ingnore it*/
> > +       snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
> > + }
> 
> and here....
> 
> > +
> > +  status_file = fopen (buf, "r");
> 
> Use gdb_fopen_cloexec?
> 
> > +  if (status_file == NULL)
> > +    {
> > +      warning (_("unable to open /proc file '%s'"), buf);
> > +      return -1;
> > +    }
> > +
> > +  while (fgets (buf, sizeof (buf), status_file))
> > +    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
> > +      {
> > +       size_t pos = field_len + 1;
> > +       while((buf[pos] == ' ' || buf[pos] == '\t')
> > +                       && (pos < sizeof (buf) - 1))
> 
> Wrong indentation.
> 
> > +         pos++;
> > +
> > +       strncpy (target, &buf[pos], t_size);
> > +       target[t_size - 1] = '\0';
> > +       retval = strlen (target);
> > +       if(retval > 0 && target[retval - 1] == '\n')
> > +               target[retval - 1] = '\0';
> 
> Wrong indentation.
> 
> 
> > diff -uNr ./gdb.org/linux-nat.c ./gdb/linux-nat.c
> > --- ./gdb.org/linux-nat.c       2013-02-13 22:59:49.000000000 +0800
> > +++ ./gdb/linux-nat.c   2013-08-16 13:22:31.000000000 +0800
> > @@ -4294,6 +4294,14 @@
> >
> >         fclose (comm_file);
> >       }
> > +  else
> > +    {
> > +      static char comm[COMM_LEN + 1];
> > +      int size = linux_proc_get_thread_name (pid, lwp, comm, COMM_LEN
> > + + 1);
> 
> A blank line is needed.
> 
> The type of pid is int and type of lwp is long, but the types of parameters of
> linux_proc_get_thread_name are pid_t and pid_t.  It is inconsistent.
> 
> > +      if (size > 0)
> > +        result = comm;
> > +
> > +    }
> >
> >   #undef COMM_LEN
> >   #undef FORMAT
> > diff -uNr ./gdb.org/testsuite/ChangeLog ./gdb/testsuite/ChangeLog
> > --- ./gdb.org/testsuite/ChangeLog       2013-04-25 20:22:26.000000000 +0800
> > +++ ./gdb/testsuite/ChangeLog   2013-08-16 11:49:02.000000000 +0800
> > @@ -1,3 +1,9 @@
> > +2013-08-14  Will Huang<will.huang@aliyun-inc.com>
> > +
> > +       PR threads/15824
> > +       * gdb.threads/manythreads.exp: Add test for get thread
> > +       original name
> 
> Period is missing.
> 
> --
> Yao (齐尧)

Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Yao Qi]

On 08/16/2013 06:22 PM, Will Huang wrote:
>    Sorry for mistakes. There is no gdb_fopen_cloexec in gdb-7.6.  Except that all comments are accepted.
>

Are you proposing a patch for GDB 7.6 tree only?  This problem exists
on mainline too.  We post patch for mainline, and backport the patch to 
internal tree or older tree.

> +
> +static int
> +linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
> +                       size_t t_size, const char *field)
> +{
> +  size_t field_len = strlen (field);
> +  FILE *status_file;
> +  char buf[100];
> +  int retval = -1;
> +
> +  if (tid > 0)
> +    xsnprintf (buf, sizeof (buf), "/proc/%d/task/%d/status",
> +               (int) lwpid, (int) tid);
> +  else
> +  {
> +     /*if TID is zero, ingnore it*/

/* If TID is zero, ignore it.  */

> +     xsnprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
> +  }
> +
> +  status_file = fopen (buf, "r");
> +  if (status_file == NULL)
> +    {
> +      warning (_("unable to open /proc file '%s'"), buf);
> +      return -1;
> +    }
> +
> +  while (fgets (buf, sizeof (buf), status_file))
> +    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
> +      {
> +       size_t pos = field_len + 1;

Blank line is needed here.

> diff -ruN ./gdb.org/linux-nat.c ./gdb/linux-nat.c
> --- ./gdb.org/linux-nat.c       2013-02-13 22:59:49.000000000 +0800
> +++ ./gdb/linux-nat.c   2013-08-16 17:55:19.000000000 +0800
> @@ -4294,6 +4294,15 @@
>
>         fclose (comm_file);
>       }
> +  else
> +    {
> +      static char comm[COMM_LEN + 1];
> +      int size = linux_proc_get_thread_name ((pid_t) pid, (pid_t) lwp, comm, COMM_LEN + 1);

This line is too long.

Otherwise, I don't have comments to this patch.

-- 
Yao (齐尧)

Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Pedro Alves]

Hi!

Thanks.  Do you have your copyright assignment on file?  We
will need it before we can accept the changes.  See
<http://sourceware.org/gdb/wiki/ContributionChecklist>.
If you haven't filed it yet, please contact me off-list and I
can send you the forms.

(I haven't really looked at the patch.)

-- 
Pedro Alves

RE: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Will Huang]

Thanks Yao,
Ok, switch to mainline. Patch is updated, please review it again. Thanks.

diff -ruN ./gdb.org/ChangeLog ./gdb/ChangeLog
--- ./gdb.org/ChangeLog 2013-08-16 12:30:22.000000000 +0800
+++ ./gdb/ChangeLog     2013-08-19 09:39:32.000000000 +0800
@@ -1,3 +1,11 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * linux-nat.c (linux_nat_thread_name): Add get thread name from
+       procfs "/proc/$pid/task/$tid/status".
+       * commo/linux-procfs.c (linux_proc_get_thread_name): New.
+       (linux_proc_get_string): New.
+
 2013-08-15  Kevin Buettner  <kevinb@redhat.com>

        * rl78-tdep.c (RL78_RAW_PC_REGNUM): New enum.
diff -ruN ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
--- ./gdb.org/common/linux-procfs.c     2013-04-23 00:46:15.000000000 +0800
+++ ./gdb/common/linux-procfs.c 2013-08-19 10:32:55.000000000 +0800
@@ -56,6 +56,57 @@
   return retval;
 }

+
+/* Return the string length of FILED /proc/LWPID/task/TID/status.
+   Return -1 if not found.  */
+
+static int
+linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
+                      size_t t_size, const char *field)
+{
+  size_t field_len = strlen (field);
+  FILE *status_file;
+  char buf[100];
+  int retval = -1;
+
+  if (tid > 0)
+    xsnprintf (buf, sizeof (buf), "/proc/%d/task/%d/status",
+              (int) lwpid, (int) tid);
+  else
+    {
+      /*if TID is zero, ingnore it */
+      xsnprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
+    }
+
+  status_file = gdb_fopen_cloexec (buf, "r");
+  if (status_file == NULL)
+    {
+      warning (_("unable to open /proc file '%s'"), buf);
+      return -1;
+    }
+
+  while (fgets (buf, sizeof (buf), status_file))
+    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
+      {
+       size_t pos = field_len + 1;
+
+       while ((buf[pos] == ' ' || buf[pos] == '\t')
+              && (pos < sizeof (buf) - 1))
+         pos++;
+
+       strncpy (target, &buf[pos], t_size);
+       target[t_size - 1] = '\0';
+       retval = strlen (target);
+       if (retval > 0 && target[retval - 1] == '\n')
+         target[retval - 1] = '\0';
+
+       break;
+      }
+
+  fclose (status_file);
+  return retval;
+}
+
 /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
    found.  */

@@ -119,3 +170,13 @@
 {
   return linux_proc_pid_has_state (pid, "Z (zombie)");
 }
+
+/* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
+   of buffer NAME.  Return the length of the name if success,
+   otherwise return -1.  */
+
+int
+linux_proc_get_thread_name (pid_t pid, pid_t tid, char *name, size_t size)
+{
+  return linux_proc_get_string (pid, tid, name, size, "Name");
+}
diff -ruN ./gdb.org/common/linux-procfs.h ./gdb/common/linux-procfs.h
--- ./gdb.org/common/linux-procfs.h     2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.h 2013-08-19 10:32:57.000000000 +0800
@@ -40,4 +40,10 @@

 extern int linux_proc_pid_is_zombie (pid_t pid);

+/* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
+   of buffer NAME.  Return the length of the name if success,
+   otherwise return -1.  */
+
+extern int linux_proc_get_thread_name (pid_t pid, pid_t tid,
+                                      char *name, size_t size);
 #endif /* COMMON_LINUX_PROCFS_H */
diff -ruN ./gdb.org/linux-nat.c ./gdb/linux-nat.c
--- ./gdb.org/linux-nat.c       2013-07-01 19:28:30.000000000 +0800
+++ ./gdb/linux-nat.c   2013-08-19 09:45:34.000000000 +0800
@@ -4291,6 +4291,17 @@
       fclose (comm_file);
     }

+  else
+    {
+      static char comm[COMM_LEN + 1];
+      int size = linux_proc_get_thread_name ((pid_t) pid, (pid_t) lwp,
+                                             comm, COMM_LEN + 1);
+
+      if (size > 0)
+        result = comm;
+
+    }
+
 #undef COMM_LEN
 #undef FORMAT

diff -ruN ./gdb.org/testsuite/ChangeLog ./gdb/testsuite/ChangeLog
--- ./gdb.org/testsuite/ChangeLog       2013-08-17 01:44:21.000000000 +0800
+++ ./gdb/testsuite/ChangeLog   2013-08-19 09:46:08.000000000 +0800
@@ -1,3 +1,9 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * gdb.threads/manythreads.exp: Add test for get thread
+       original name.
+
 2013-08-16  Doug Evans  <dje@google.com>

        * lib/prelink-support.exp (prelink_yes): Flag test as unsupported if
diff -ruN ./gdb.org/testsuite/gdb.threads/manythreads.exp ./gdb/testsuite/gdb.threads/manythreads.exp
--- ./gdb.org/testsuite/gdb.threads/manythreads.exp     2013-01-01 14:41:27.000000000 +0800
+++ ./gdb/testsuite/gdb.threads/manythreads.exp 2013-08-19 09:47:10.000000000 +0800
@@ -93,6 +93,7 @@
     }
 }

+gdb_test "info threads" ".*manythreads.*" "check thread original name"
 gdb_test_no_output "thread name zardoz" "give a name to the thread"
 gdb_test "info threads" ".*zardoz.*" "check thread name"



Thanks,
Will.huang


> -----Original Message-----
> From: Yao Qi [mailto:yao@codesourcery.com]
> Sent: Friday, August 16, 2013 7:29 PM
> To: Will Huang
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH][PR threads/15824] when get threads name failed from info
> threads with linux kernel version earlier than 2.6.33
> 
> On 08/16/2013 06:22 PM, Will Huang wrote:
> >    Sorry for mistakes. There is no gdb_fopen_cloexec in gdb-7.6.  Except that
> all comments are accepted.
> >
> 
> Are you proposing a patch for GDB 7.6 tree only?  This problem exists on
> mainline too.  We post patch for mainline, and backport the patch to internal
> tree or older tree.
> 
> > +
> > +static int
> > +linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
> > +                       size_t t_size, const char *field) {
> > +  size_t field_len = strlen (field);
> > +  FILE *status_file;
> > +  char buf[100];
> > +  int retval = -1;
> > +
> > +  if (tid > 0)
> > +    xsnprintf (buf, sizeof (buf), "/proc/%d/task/%d/status",
> > +               (int) lwpid, (int) tid);  else  {
> > +     /*if TID is zero, ingnore it*/
> 
> /* If TID is zero, ignore it.  */
> 
> > +     xsnprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
> > + }
> > +
> > +  status_file = fopen (buf, "r");
> > +  if (status_file == NULL)
> > +    {
> > +      warning (_("unable to open /proc file '%s'"), buf);
> > +      return -1;
> > +    }
> > +
> > +  while (fgets (buf, sizeof (buf), status_file))
> > +    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
> > +      {
> > +       size_t pos = field_len + 1;
> 
> Blank line is needed here.
> 
> > diff -ruN ./gdb.org/linux-nat.c ./gdb/linux-nat.c
> > --- ./gdb.org/linux-nat.c       2013-02-13 22:59:49.000000000 +0800
> > +++ ./gdb/linux-nat.c   2013-08-16 17:55:19.000000000 +0800
> > @@ -4294,6 +4294,15 @@
> >
> >         fclose (comm_file);
> >       }
> > +  else
> > +    {
> > +      static char comm[COMM_LEN + 1];
> > +      int size = linux_proc_get_thread_name ((pid_t) pid, (pid_t)
> > + lwp, comm, COMM_LEN + 1);
> 
> This line is too long.
> 
> Otherwise, I don't have comments to this patch.
> 
> --
> Yao (齐尧)

Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Yao Qi]

On 08/19/2013 10:52 AM, Will Huang wrote:
> +
> +/* Return the string length of FILED /proc/LWPID/task/TID/status.
> +   Return -1 if not found.  */
> +
> +static int
> +linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
> +                      size_t t_size, const char *field)


I suggest renaming this function to "linux_proc_parse_status" or 
something else, and adjust the comment.

/* Parse file /proc/LWPID/task/TID/status, save the value of FIELD
    in buffer TARGET, whose length is T_SIZE, and return the length
    of the value.  If TID is zero, parse /proc/LWPID/status instead.
    Return -1 if not found.  */

static int
linux_proc_parse_status (pid_t lwpid, pid_t tid, char *target,
                          size_t t_size, const char *field)
-- 
Yao (齐尧)

RE: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33

[Will Huang]

Yes, inux_proc_parse_status is more straightforward than linux_proc_get_string.
Changes updated.


diff -ruN ./gdb.org/ChangeLog ./gdb/ChangeLog
--- ./gdb.org/ChangeLog 2013-08-16 12:30:22.000000000 +0800
+++ ./gdb/ChangeLog     2013-08-19 11:45:35.000000000 +0800
@@ -1,3 +1,11 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * linux-nat.c (linux_nat_thread_name): Add get thread name from
+       procfs "/proc/$pid/task/$tid/status".
+       * commo/linux-procfs.c (linux_proc_get_thread_name): New.
+       (linux_proc_parse_status): New.
+
 2013-08-15  Kevin Buettner  <kevinb@redhat.com>

        * rl78-tdep.c (RL78_RAW_PC_REGNUM): New enum.
diff -ruN ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
--- ./gdb.org/common/linux-procfs.c     2013-04-23 00:46:15.000000000 +0800
+++ ./gdb/common/linux-procfs.c 2013-08-19 11:46:36.000000000 +0800
@@ -56,6 +56,58 @@
   return retval;
 }

+/* Parse file /proc/LWPID/task/TID/status, save the value of FIELD
+   in buffer TARGET, whose length is T_SIZE, and return the length
+   of the value.  If TID is zero, parse /proc/LWPID/status instead.
+   Return -1 if not found.  */
+
+static int
+linux_proc_parse_status (pid_t lwpid, pid_t tid, char *target,
+                      size_t t_size, const char *field)
+{
+  size_t field_len = strlen (field);
+  FILE *status_file;
+  char buf[100];
+  int retval = -1;
+
+  if (tid > 0)
+    xsnprintf (buf, sizeof (buf), "/proc/%d/task/%d/status",
+              (int) lwpid, (int) tid);
+  else
+    {
+      /*if TID is zero, ingnore it */
+      xsnprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
+    }
+
+  status_file = gdb_fopen_cloexec (buf, "r");
+  if (status_file == NULL)
+    {
+      warning (_("unable to open /proc file '%s'"), buf);
+      return -1;
+    }
+
+  while (fgets (buf, sizeof (buf), status_file))
+    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
+      {
+       size_t pos = field_len + 1;
+
+       while ((buf[pos] == ' ' || buf[pos] == '\t')
+              && (pos < sizeof (buf) - 1))
+         pos++;
+
+       strncpy (target, &buf[pos], t_size);
+       target[t_size - 1] = '\0';
+       retval = strlen (target);
+       if (retval > 0 && target[retval - 1] == '\n')
+         target[retval - 1] = '\0';
+
+       break;
+      }
+
+  fclose (status_file);
+  return retval;
+}
+
 /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
    found.  */

@@ -119,3 +171,13 @@
 {
   return linux_proc_pid_has_state (pid, "Z (zombie)");
 }
+
+/* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
+   of buffer NAME.  Return the length of the name if success,
+   otherwise return -1.  */
+
+int
+linux_proc_get_thread_name (pid_t pid, pid_t tid, char *name, size_t size)
+{
+  return linux_proc_parse_status (pid, tid, name, size, "Name");
+}
diff -ruN ./gdb.org/common/linux-procfs.h ./gdb/common/linux-procfs.h
--- ./gdb.org/common/linux-procfs.h     2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.h 2013-08-19 10:32:57.000000000 +0800
@@ -40,4 +40,10 @@

 extern int linux_proc_pid_is_zombie (pid_t pid);

+/* Save the name of thread PID/TID into buffer NAME.  SIZE is the size
+   of buffer NAME.  Return the length of the name if success,
+   otherwise return -1.  */
+
+extern int linux_proc_get_thread_name (pid_t pid, pid_t tid,
+                                      char *name, size_t size);
 #endif /* COMMON_LINUX_PROCFS_H */
diff -ruN ./gdb.org/linux-nat.c ./gdb/linux-nat.c
--- ./gdb.org/linux-nat.c       2013-07-01 19:28:30.000000000 +0800
+++ ./gdb/linux-nat.c   2013-08-19 09:45:34.000000000 +0800
@@ -4291,6 +4291,17 @@
       fclose (comm_file);
     }

+  else
+    {
+      static char comm[COMM_LEN + 1];
+      int size = linux_proc_get_thread_name ((pid_t) pid, (pid_t) lwp,
+                                             comm, COMM_LEN + 1);
+
+      if (size > 0)
+        result = comm;
+
+    }
+
 #undef COMM_LEN
 #undef FORMAT

diff -ruN ./gdb.org/testsuite/ChangeLog ./gdb/testsuite/ChangeLog
--- ./gdb.org/testsuite/ChangeLog       2013-08-17 01:44:21.000000000 +0800
+++ ./gdb/testsuite/ChangeLog   2013-08-19 09:46:08.000000000 +0800
@@ -1,3 +1,9 @@
+2013-08-14  Will Huang  <will.huang@aliyun-inc.com>
+
+       PR threads/15824
+       * gdb.threads/manythreads.exp: Add test for get thread
+       original name.
+
 2013-08-16  Doug Evans  <dje@google.com>

        * lib/prelink-support.exp (prelink_yes): Flag test as unsupported if
diff -ruN ./gdb.org/testsuite/gdb.threads/manythreads.exp ./gdb/testsuite/gdb.threads/manythreads.exp
--- ./gdb.org/testsuite/gdb.threads/manythreads.exp     2013-01-01 14:41:27.000000000 +0800
+++ ./gdb/testsuite/gdb.threads/manythreads.exp 2013-08-19 09:47:10.000000000 +0800
@@ -93,6 +93,7 @@
     }
 }

+gdb_test "info threads" ".*manythreads.*" "check thread original name"
 gdb_test_no_output "thread name zardoz" "give a name to the thread"
 gdb_test "info threads" ".*zardoz.*" "check thread name"



Thanks,
Will Huang

> -----Original Message-----
> From: Yao Qi [mailto:yao@codesourcery.com]
> Sent: Monday, August 19, 2013 11:10 AM
> To: Will Huang
> Cc: palves@redhat.com; gdb-patches@sourceware.org
> Subject: Re: [PATCH][PR threads/15824] when get threads name failed from info
> threads with linux kernel version earlier than 2.6.33
> 
> On 08/19/2013 10:52 AM, Will Huang wrote:
> > +
> > +/* Return the string length of FILED /proc/LWPID/task/TID/status.
> > +   Return -1 if not found.  */
> > +
> > +static int
> > +linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
> > +                      size_t t_size, const char *field)
> 
> 
> I suggest renaming this function to "linux_proc_parse_status" or something else,
> and adjust the comment.
> 
> /* Parse file /proc/LWPID/task/TID/status, save the value of FIELD
>     in buffer TARGET, whose length is T_SIZE, and return the length
>     of the value.  If TID is zero, parse /proc/LWPID/status instead.
>     Return -1 if not found.  */
> 
> static int
> linux_proc_parse_status (pid_t lwpid, pid_t tid, char *target,
>                           size_t t_size, const char *field)
> --
> Yao (齐尧)