From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12210 invoked by alias); 18 Mar 2014 10:14:07 -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 12198 invoked by uid 89); 18 Mar 2014 10:14:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Mar 2014 10:14:05 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s2IAE3Qh025059 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 18 Mar 2014 06:14:03 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s2IAE2ku030094; Tue, 18 Mar 2014 06:14:02 -0400 Message-ID: <53281C69.7090703@redhat.com> Date: Tue, 18 Mar 2014 10:14:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Hui Zhu CC: gdb-patches ml Subject: Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx" References: <53271C09.5000709@mentor.com> <53271ED2.6010707@redhat.com> <53272958.1080605@redhat.com> <5327F7D0.1050304@mentor.com> In-Reply-To: <5327F7D0.1050304@mentor.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-03/txt/msg00410.txt.bz2 On 03/18/2014 07:37 AM, Hui Zhu wrote: > On 03/18/14 00:56, Pedro Alves wrote: > According to your comments, I make a new patch that change all > methods return a static buffer. Thank you. >> Bummer that we don't have a test that caught this. :-( >> > > Yes, I found it when I did regression test. WDYM? What test failed then? > Does testsuite have some example to test a GDB feature like "gdb -p"? I guess we'd refactor attach.exp a little to test that in addition to "attach", on native targets. And I supposed we could pass down the -p to gdb by tweaking GDBFLAGS, like several tests do. gdb.base/corefile.exp sounds like the model to follow, as that spawns "gdb -core", which is very similar to "gdb -p". > 2014-03-18 Hui Zhu > > * darwin-nat.c (darwin_pid_to_exec_file): Change xmalloc to > static buffer. > * fbsd-nat.c (fbsd_pid_to_exec_file): Ditto. > * linux-nat.c (linux_child_pid_to_exec_file): Ditto. > * nbsd-nat.c (nbsd_pid_to_exec_file): Ditto. > > --- a/gdb/darwin-nat.c > +++ b/gdb/darwin-nat.c > @@ -1991,12 +1991,9 @@ set_enable_mach_exceptions (char *args, > static char * > darwin_pid_to_exec_file (struct target_ops *self, int pid) > { > - char *path; > + static char path[PATH_MAX]; > int res; > > - path = xmalloc (PATH_MAX); > - make_cleanup (xfree, path); > - > res = proc_pidinfo (pid, PROC_PIDPATHINFO, 0, path, PATH_MAX); > if (res >= 0) > return path; > --- a/gdb/fbsd-nat.c > +++ b/gdb/fbsd-nat.c > @@ -40,8 +40,8 @@ char * > fbsd_pid_to_exec_file (struct target_ops *self, int pid) > { > size_t len = PATH_MAX; > - char *buf = xcalloc (len, sizeof (char)); > - char *path; > + static char buf[PATH_MAX]; > + char *path, *ret; > > #ifdef KERN_PROC_PATHNAME > int mib[4]; > @@ -56,13 +56,12 @@ fbsd_pid_to_exec_file (struct target_ops > > path = xstrprintf ("/proc/%d/file", pid); > if (readlink (path, buf, PATH_MAX - 1) == -1) readlink does not '\0' terminate, we need to do that ourselves. See below. > - { > - xfree (buf); > - buf = NULL; > - } > + ret = NULL; > + else > + ret = buf; > > xfree (path); > - return buf; > + return ret; > } > > static int > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -4011,19 +4011,18 @@ linux_nat_thread_name (struct target_ops > static char * > linux_child_pid_to_exec_file (struct target_ops *self, int pid) > { > - char *name1, *name2; > + static char buf[PATH_MAX]; > + char name1[PATH_MAX], name2[PATH_MAX]; > > - name1 = xmalloc (PATH_MAX); > - name2 = xmalloc (PATH_MAX); > - make_cleanup (xfree, name1); > - make_cleanup (xfree, name2); > memset (name2, 0, PATH_MAX); > > xsnprintf (name1, PATH_MAX, "/proc/%d/exe", pid); > if (readlink (name1, name2, PATH_MAX - 1) > 0) > - return name2; > + strcpy (buf, name2); > else > - return name1; > + strcpy (buf, name1); > + > + return buf; No need for three buffers. AFAICS, this should suffice: static char buf[PATH_MAX]; char name[PATH_MAX]; xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid); memset (buf, 0, PATH_MAX); if (readlink (name, buf, PATH_MAX - 1) <= 0) strcpy (buf, name); return buf; > } > > /* Records the thread's register state for the corefile note > --- a/gdb/nbsd-nat.c > +++ b/gdb/nbsd-nat.c > @@ -28,16 +28,15 @@ char * > nbsd_pid_to_exec_file (struct target_ops *self, int pid) > { > size_t len = PATH_MAX; > - char *buf = xcalloc (len, sizeof (char)); > - char *path; > + static char buf[PATH_MAX]; > + char *path, *ret; > > path = xstrprintf ("/proc/%d/exe", pid); > if (readlink (path, buf, PATH_MAX - 1) == -1) > - { > - xfree (buf); > - buf = NULL; > - } > + ret = NULL; > + else > + ret = buf; > > xfree (path); > - return buf; > + return ret; > } Same with the nul termination. The most standard solution is: static char buf[PATH_MAX]; char name[PATH_MAX]; ssize_t len; xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid); len = readlink (name, buf, PATH_MAX - 1); if (len != -1) { buf[len] = '\0'; return buf; } return NULL; -- Pedro Alves