* [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33
@ 2013-08-14 4:27 Will Huang
2013-08-16 2:26 ` Yao Qi
0 siblings, 1 reply; 10+ messages in thread
From: Will Huang @ 2013-08-14 4:27 UTC (permalink / raw)
To: gdb-patches; +Cc: '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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 2013-08-14 4:27 [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 Will Huang @ 2013-08-16 2:26 ` Yao Qi 2013-08-16 5:32 ` Will Huang 0 siblings, 1 reply; 10+ messages in thread From: Yao Qi @ 2013-08-16 2:26 UTC (permalink / raw) To: Will Huang; +Cc: gdb-patches 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 (é½å°§) ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 2013-08-16 2:26 ` Yao Qi @ 2013-08-16 5:32 ` Will Huang 2013-08-16 9:38 ` Yao Qi 0 siblings, 1 reply; 10+ messages in thread From: Will Huang @ 2013-08-16 5:32 UTC (permalink / raw) To: 'Yao Qi'; +Cc: gdb-patches 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 (齐尧) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 2013-08-16 5:32 ` Will Huang @ 2013-08-16 9:38 ` Yao Qi 2013-08-16 10:22 ` Will Huang 0 siblings, 1 reply; 10+ messages in thread From: Yao Qi @ 2013-08-16 9:38 UTC (permalink / raw) To: Will Huang; +Cc: gdb-patches 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 (é½å°§) ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 2013-08-16 9:38 ` Yao Qi @ 2013-08-16 10:22 ` Will Huang 2013-08-16 11:29 ` Yao Qi 0 siblings, 1 reply; 10+ messages in thread From: Will Huang @ 2013-08-16 10:22 UTC (permalink / raw) To: 'Yao Qi'; +Cc: gdb-patches 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 (齐尧) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 2013-08-16 10:22 ` Will Huang @ 2013-08-16 11:29 ` Yao Qi 2013-08-16 16:24 ` Pedro Alves 2013-08-19 2:52 ` Will Huang 0 siblings, 2 replies; 10+ messages in thread From: Yao Qi @ 2013-08-16 11:29 UTC (permalink / raw) To: Will Huang; +Cc: gdb-patches 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 (é½å°§) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 2013-08-16 11:29 ` Yao Qi @ 2013-08-16 16:24 ` Pedro Alves 2013-08-19 2:52 ` Will Huang 1 sibling, 0 replies; 10+ messages in thread From: Pedro Alves @ 2013-08-16 16:24 UTC (permalink / raw) To: Will Huang; +Cc: Yao Qi, gdb-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 2013-08-16 11:29 ` Yao Qi 2013-08-16 16:24 ` Pedro Alves @ 2013-08-19 2:52 ` Will Huang 2013-08-19 3:10 ` Yao Qi 1 sibling, 1 reply; 10+ messages in thread From: Will Huang @ 2013-08-19 2:52 UTC (permalink / raw) To: 'Yao Qi', palves; +Cc: gdb-patches 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 (齐尧) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 2013-08-19 2:52 ` Will Huang @ 2013-08-19 3:10 ` Yao Qi 2013-08-19 5:01 ` Will Huang 0 siblings, 1 reply; 10+ messages in thread From: Yao Qi @ 2013-08-19 3:10 UTC (permalink / raw) To: Will Huang; +Cc: palves, gdb-patches 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 (é½å°§) ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 2013-08-19 3:10 ` Yao Qi @ 2013-08-19 5:01 ` Will Huang 0 siblings, 0 replies; 10+ messages in thread From: Will Huang @ 2013-08-19 5:01 UTC (permalink / raw) To: 'Yao Qi'; +Cc: palves, gdb-patches 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 (齐尧) ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-08-19 5:01 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-08-14 4:27 [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 Will Huang 2013-08-16 2:26 ` Yao Qi 2013-08-16 5:32 ` Will Huang 2013-08-16 9:38 ` Yao Qi 2013-08-16 10:22 ` Will Huang 2013-08-16 11:29 ` Yao Qi 2013-08-16 16:24 ` Pedro Alves 2013-08-19 2:52 ` Will Huang 2013-08-19 3:10 ` Yao Qi 2013-08-19 5:01 ` Will Huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox