Re: [RFA] c++/13615

[Keith Seitz <keiths@redhat.com>]

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