From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13703 invoked by alias); 16 Aug 2013 09:38:38 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 13694 invoked by uid 89); 16 Aug 2013 09:38:37 -0000 X-Spam-SWARE-Status: No, score=-4.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL autolearn=ham version=3.3.2 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 16 Aug 2013 09:38:36 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1VAGUM-00049Q-3J from Yao_Qi@mentor.com ; Fri, 16 Aug 2013 02:38:34 -0700 Received: from SVR-ORW-FEM-03.mgc.mentorg.com ([147.34.97.39]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Fri, 16 Aug 2013 02:38:33 -0700 Received: from qiyao.dyndns.org (147.34.91.1) by svr-orw-fem-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server id 14.2.247.3; Fri, 16 Aug 2013 02:38:32 -0700 Message-ID: <520DF2E2.3020407@codesourcery.com> Date: Fri, 16 Aug 2013 09:38:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Will Huang CC: Subject: Re: [PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33 References: <00b201ce98a6$7cf69150$76e3b3f0$@huang@aliyun-inc.com> <520D8D9A.3050003@codesourcery.com> <016701ce9a41$f39086d0$dab19470$@huang@aliyun-inc.com> In-Reply-To: <016701ce9a41$f39086d0$dab19470$@huang@aliyun-inc.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2013-08/txt/msg00417.txt.bz2 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 > + > + 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 > > * 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 > + > + PR threads/15824 > + * gdb.threads/manythreads.exp: Add test for get thread > + original name Period is missing. -- Yao (齐尧)