This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"
- From: Hui Zhu <hui_zhu at mentor dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches ml <gdb-patches at sourceware dot org>
- Date: Wed, 19 Mar 2014 11:57:51 +0800
- Subject: Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"
- Authentication-results: sourceware.org; auth=none
- References: <53271C09 dot 5000709 at mentor dot com> <53271ED2 dot 6010707 at redhat dot com> <53272958 dot 1080605 at redhat dot com> <5327F7D0 dot 1050304 at mentor dot com> <53281C69 dot 7090703 at redhat dot com>
On 03/18/14 18:14, Pedro Alves wrote:
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?
Sorry, my mean is I found the issue cannot be found by regression test.
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".
I make a patch for that. I will post it in a separate mail.
2014-03-18 Hui Zhu <hui@codesourcery.com>
* 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;
I make a new patch according to your comments.
Please help me review it.
Thanks,
Hui
2014-03-19 Hui Zhu <hui@codesourcery.com>
Pedro Alves <palves@redhat.com>
* 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 name[PATH_MAX];
#ifdef KERN_PROC_PATHNAME
int mib[4];
@@ -54,14 +54,11 @@ fbsd_pid_to_exec_file (struct target_ops
return buf;
#endif
- path = xstrprintf ("/proc/%d/file", pid);
- if (readlink (path, buf, PATH_MAX - 1) == -1)
- {
- xfree (buf);
- buf = NULL;
- }
+ xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+ memset (buf, 0, PATH_MAX);
+ if (readlink (name, buf, PATH_MAX - 1) <= 0)
+ strcpy (buf, name);
- xfree (path);
return buf;
}
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4011,19 +4011,15 @@ 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 name[PATH_MAX];
- name1 = xmalloc (PATH_MAX);
- name2 = xmalloc (PATH_MAX);
- make_cleanup (xfree, name1);
- make_cleanup (xfree, name2);
- memset (name2, 0, 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);
- xsnprintf (name1, PATH_MAX, "/proc/%d/exe", pid);
- if (readlink (name1, name2, PATH_MAX - 1) > 0)
- return name2;
- else
- return name1;
+ return buf;
}
/* Records the thread's register state for the corefile note
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -27,17 +27,13 @@
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 name[PATH_MAX];
- path = xstrprintf ("/proc/%d/exe", pid);
- if (readlink (path, buf, PATH_MAX - 1) == -1)
- {
- xfree (buf);
- buf = NULL;
- }
+ xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+ memset (buf, 0, PATH_MAX);
+ if (readlink (name, buf, PATH_MAX - 1) <= 0)
+ strcpy (buf, name);
- xfree (path);
return buf;
}