This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFA] fixes to xmalloc for gdbserver patch
- From: dje at google dot com (Doug Evans)
- To: gdb-patches at sourceware dot org
- Date: Tue, 16 Dec 2008 00:43:09 -0800 (PST)
- Subject: [RFA] fixes to xmalloc for gdbserver patch
Oops. I forgot to watch for calls to malloc that are checked
and report failure back to gdb.
This add a couple more checks for malloc failure and reporting
the failure back to gdb instead of dying, as well as a couple
of fixmes for some minor leaks.
It also adds a freeargv function which is used in server.c.
Fixing some of the memory leaks could use it too, except
a bit more work is required because new_argv[0] can be NULL
(which would cause freeargv to miss the later elements).
Ok to check in?
2008-12-16 Doug Evans <dje@google.com>
* regcache.c (new_register_cache): No need to check result of xcalloc.
* server.c (handle_search_memory): Back out calls to xmalloc,
result is checked and error is returned to user upon failure.
(handle_query): Ditto. Add more checks for result of malloc.
(handle_v_cont): Check result of malloc, report error back to
user upon failure.
(handle_v_run): Ditto. Call freeargv.
* server.h (freeargv): Declare.
* utils.c (freeargv): New fn.
Index: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/regcache.c,v
retrieving revision 1.16
diff -u -p -r1.16 regcache.c
--- regcache.c 14 Dec 2008 20:51:04 -0000 1.16
+++ regcache.c 16 Dec 2008 08:29:35 -0000
@@ -100,8 +100,6 @@ new_register_cache (void)
in case there are registers the target never fetches. This way they'll
read as zero instead of garbage. */
regcache->registers = xcalloc (1, register_bytes);
- if (regcache->registers == NULL)
- fatal ("Could not allocate register cache.");
regcache->registers_valid = 0;
Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.82
diff -u -p -r1.82 server.c
--- server.c 14 Dec 2008 20:51:04 -0000 1.82
+++ server.c 16 Dec 2008 08:29:35 -0000
@@ -436,7 +436,7 @@ handle_search_memory (char *own_buf, int
CORE_ADDR found_addr;
int cmd_name_len = sizeof ("qSearch:memory:") - 1;
- pattern = xmalloc (packet_len);
+ pattern = malloc (packet_len);
if (pattern == NULL)
{
error ("Unable to allocate memory to perform the search");
@@ -460,7 +460,7 @@ handle_search_memory (char *own_buf, int
if (search_space_len < search_buf_size)
search_buf_size = search_space_len;
- search_buf = xmalloc (search_buf_size);
+ search_buf = malloc (search_buf_size);
if (search_buf == NULL)
{
free (pattern);
@@ -575,7 +575,7 @@ handle_query (char *own_buf, int packet_
return;
if (len > PBUFSIZ - 2)
len = PBUFSIZ - 2;
- spu_buf = xmalloc (len + 1);
+ spu_buf = malloc (len + 1);
if (!spu_buf)
return;
@@ -604,7 +604,7 @@ handle_query (char *own_buf, int packet_
require_running (own_buf);
strcpy (own_buf, "E00");
- spu_buf = xmalloc (packet_len - 15);
+ spu_buf = malloc (packet_len - 15);
if (!spu_buf)
return;
if (decode_xfer_write (own_buf + 16, packet_len - 16, &annex,
@@ -648,7 +648,12 @@ handle_query (char *own_buf, int packet_
more. */
if (len > PBUFSIZ - 2)
len = PBUFSIZ - 2;
- data = xmalloc (len + 1);
+ data = malloc (len + 1);
+ if (data == NULL)
+ {
+ write_enn (own_buf);
+ return;
+ }
n = (*the_target->read_auxv) (ofs, data, len + 1);
if (n < 0)
write_enn (own_buf);
@@ -726,7 +731,12 @@ handle_query (char *own_buf, int packet_
for (dll_ptr = all_dlls.head; dll_ptr != NULL; dll_ptr = dll_ptr->next)
total_len += 128 + 6 * strlen (((struct dll_info *) dll_ptr)->name);
- document = xmalloc (total_len);
+ document = malloc (total_len);
+ if (document == NULL)
+ {
+ write_enn (own_buf);
+ return;
+ }
strcpy (document, "<library-list>\n");
p = document + strlen (document);
@@ -782,7 +792,7 @@ handle_query (char *own_buf, int packet_
return;
if (len > PBUFSIZ - 2)
len = PBUFSIZ - 2;
- workbuf = xmalloc (len + 1);
+ workbuf = malloc (len + 1);
if (!workbuf)
return;
@@ -895,9 +905,15 @@ handle_query (char *own_buf, int packet_
/* Handle "monitor" commands. */
if (strncmp ("qRcmd,", own_buf, 6) == 0)
{
- char *mon = xmalloc (PBUFSIZ);
+ char *mon = malloc (PBUFSIZ);
int len = strlen (own_buf + 6);
+ if (mon == NULL)
+ {
+ write_enn (own_buf);
+ return;
+ }
+
if ((len % 2) != 0 || unhexify (mon, own_buf + 6, len / 2) != len / 2)
{
write_enn (own_buf);
@@ -975,7 +991,9 @@ handle_v_cont (char *own_buf, char *stat
/* Allocate room for one extra action, for the default remain-stopped
behavior; if no default action is in the list, we'll need the extra
slot. */
- resume_info = xmalloc ((n + 1) * sizeof (resume_info[0]));
+ resume_info = malloc ((n + 1) * sizeof (resume_info[0]));
+ if (resume_info == NULL)
+ goto err;
default_action.thread = -1;
default_action.leave_stopped = 1;
@@ -1097,7 +1115,7 @@ handle_v_attach (char *own_buf, char *st
static int
handle_v_run (char *own_buf, char *status, int *signal)
{
- char *p, **pp, *next_p, **new_argv;
+ char *p, *next_p, **new_argv;
int i, new_argc;
new_argc = 0;
@@ -1107,7 +1125,13 @@ handle_v_run (char *own_buf, char *statu
new_argc++;
}
- new_argv = xcalloc (new_argc + 2, sizeof (char *));
+ new_argv = calloc (new_argc + 2, sizeof (char *));
+ if (new_argv == NULL)
+ {
+ write_enn (own_buf);
+ return 0;
+ }
+
i = 0;
for (p = own_buf + strlen ("vRun;"); *p; p = next_p)
{
@@ -1119,6 +1143,7 @@ handle_v_run (char *own_buf, char *statu
new_argv[i] = NULL;
else
{
+ /* FIXME: Fail request if out of memory instead of dying. */
new_argv[i] = xmalloc (1 + (next_p - p) / 2);
unhexify (new_argv[i], p, (next_p - p) / 2);
new_argv[i][(next_p - p) / 2] = '\0';
@@ -1137,20 +1162,22 @@ handle_v_run (char *own_buf, char *statu
if (program_argv == NULL)
{
+ /* FIXME: new_argv memory leak */
write_enn (own_buf);
return 0;
}
- new_argv[0] = xstrdup (program_argv[0]);
+ new_argv[0] = strdup (program_argv[0]);
+ if (new_argv[0] == NULL)
+ {
+ /* FIXME: new_argv memory leak */
+ write_enn (own_buf);
+ return 0;
+ }
}
- /* Free the old argv. */
- if (program_argv)
- {
- for (pp = program_argv; *pp != NULL; pp++)
- free (*pp);
- free (program_argv);
- }
+ /* Free the old argv and install the new one. */
+ freeargv (program_argv);
program_argv = new_argv;
*signal = start_inferior (program_argv, status);
Index: server.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.h,v
retrieving revision 1.48
diff -u -p -r1.48 server.h
--- server.h 14 Dec 2008 20:51:04 -0000 1.48
+++ server.h 16 Dec 2008 08:29:35 -0000
@@ -276,6 +276,7 @@ char *target_signal_to_name (enum target
void *xmalloc (size_t) ATTR_MALLOC;
void *xcalloc (size_t, size_t) ATTR_MALLOC;
char *xstrdup (const char *) ATTR_MALLOC;
+void freeargv (char **argv);
void perror_with_name (char *string);
void error (const char *string,...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2);
void fatal (const char *string,...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2);
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/utils.c,v
retrieving revision 1.15
diff -u -p -r1.15 utils.c
--- utils.c 14 Dec 2008 20:51:04 -0000 1.15
+++ utils.c 16 Dec 2008 08:29:35 -0000
@@ -87,6 +87,23 @@ xstrdup (const char *s)
return ret;
}
+/* Free a standard argv vector. */
+
+void
+freeargv (char **vector)
+{
+ char **scan;
+
+ if (vector != NULL)
+ {
+ for (scan = vector; *scan != NULL; scan++)
+ {
+ free (*scan);
+ }
+ free (vector);
+ }
+}
+
/* Print the system error message for errno, and also mention STRING
as the file name for which the error was encountered.
Then return to command level. */