This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] remote.c, clean-up mix-up
On Thursday 06 May 2010 00:08:20, Pedro Alves wrote:
> A null_cleanup's intention is always to start a new
> cleanup scope. If the variable is unused, it either
> means the new cleanup scope wasn't neecessary in the
> first place, or, there's a do_cleanups call missing
> for this inner cleanups scope.
>
> As I said in the other email, there appears to be
> a bug here that may hint at what was meant. See the
> other uses of gdb_xml_create_parser_and_cleanup in
> the tree:
...
>
> Note how all of them install a cleanup for deleting
> the result of the parsing, in case parsing fails midway,
> when pieces of the result had already been allocated.
Here's what I'm testing.
--
Pedro Alves
2010-05-05 Pedro Alves <pedro@codesourcery.com>
* remote.c (clear_threads_parsing_context): New.
(remote_threads_info): Delele unused null_cleanup. Install a
cleanup to clear the threads_parsing_context in case parsing
throws.
---
gdb/remote.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c 2010-05-06 00:35:10.000000000 +0100
+++ src/gdb/remote.c 2010-05-06 00:41:27.000000000 +0100
@@ -2482,6 +2482,21 @@ const struct gdb_xml_element threads_ele
{ NULL, NULL, NULL, GDB_XML_EF_NONE, NULL, NULL }
};
+/* Discard the contents of the constructed thread info context. */
+
+static void
+clear_threads_parsing_context (void *p)
+{
+ struct threads_parsing_context *context = p;
+ int i;
+ struct thread_item *item;
+
+ for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
+ xfree (item->extra);
+
+ VEC_free (thread_item_t, context->items);
+}
+
#endif
/*
@@ -2512,7 +2527,7 @@ remote_threads_info (struct target_ops *
{
struct gdb_xml_parser *parser;
struct threads_parsing_context context;
- struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
+ struct cleanup *clear_parsing_context;
context.items = 0;
parser = gdb_xml_create_parser_and_cleanup (_("threads"),
@@ -2521,6 +2536,9 @@ remote_threads_info (struct target_ops *
gdb_xml_use_dtd (parser, "threads.dtd");
+ clear_parsing_context
+ = make_cleanup (clear_threads_parsing_context, &context);
+
if (gdb_xml_parse (parser, xml) == 0)
{
int i;
@@ -2542,13 +2560,12 @@ remote_threads_info (struct target_ops *
info = demand_private_info (item->ptid);
info->core = item->core;
info->extra = item->extra;
- item->extra = 0;
+ item->extra = NULL;
}
- xfree (item->extra);
}
}
- VEC_free (thread_item_t, context.items);
+ do_cleanups (clear_parsing_context);
}
do_cleanups (back_to);