This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] c++/13615


On 11/15/2012 01:59 PM, Tom Tromey wrote:
Keith> +  cleanup = make_cleanup (null_cleanup, NULL);
Keith> +  for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i)
Keith> +    {
Keith> +      const char *base_name = TYPE_BASECLASS_NAME (parent_type, i);

First, can this ever be NULL?

The comments around the declaration say it could be NULL, so I've added a guard against that. [The only two other uses of this macro in stabsread.c and valops.c both guard against this, too.]


Keith> +      discard_cleanups (cleanup);
Keith> +      concatenated_name = xrealloc (concatenated_name,
Keith> +				    (strlen (base_name) + 2
Keith> +				     + strlen (name) + 1));
Keith> +      cleanup = make_cleanup (xfree, concatenated_name);

Second, I was confused by this code the first time through -- discarding
the cleanup and then re-creating it is a bit unusual.
I think it would be simpler & cleaner to do something like:

*SMACK* Duh. I seldom seem to use/remember free_current_contents!


Below is the changes I've made to the patch. [I apologize if this is a bit unorthodox, but the changes are pretty small. Ping me if you want me to repost the patch in its entirety.]

Keith

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index f2f79ea..deb791e 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -741,22 +741,23 @@ find_symbol_in_baseclass (struct type *parent_type, const char *name,


   sym = NULL;
   concatenated_name = NULL;
-  cleanup = make_cleanup (null_cleanup, NULL);
+  cleanup = make_cleanup (free_current_contents, &concatenated_name);
   for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i)
     {
+      size_t len;
       const char *base_name = TYPE_BASECLASS_NAME (parent_type, i);

+ if (base_name == NULL)
+ continue;
+
/* Search this particular base class. */
sym = cp_lookup_symbol_namespace (base_name, name, block, VAR_DOMAIN);
if (sym != NULL)
break;


-      discard_cleanups (cleanup);
-      concatenated_name = xrealloc (concatenated_name,
-				    (strlen (base_name) + 2
-				     + strlen (name) + 1));
-      cleanup = make_cleanup (xfree, concatenated_name);
-      sprintf (concatenated_name, "%s::%s", base_name, name);
+      len = strlen (base_name) + 2 + strlen (name) + 1;
+      concatenated_name = xrealloc (concatenated_name,len);
+      xsnprintf (concatenated_name, len, "%s::%s", base_name, name);
       sym = lookup_symbol_static (concatenated_name, block, VAR_DOMAIN);

/* If there is currently no BLOCK, e.g., the inferior hasn't yet


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]