Re: [RFA] c++/13615

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> I've added a test for this and updated the patch to correct that bug.

Thanks.

Keith> ChangeLog
Keith> 2012-11-14  Keith Seitz  <keiths@redhat.com>
Keith> 	c++/13615

The commit message parser needs to see the text "PR" in there:

	PR c++/13615

Keith> +static struct symbol *
Keith> +find_symbol_in_baseclass (struct type *parent_type, const char *name,
Keith> +			   const struct block *block)
Keith> +{

Three minor notes here...

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?

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:

char *concatenated_name = NULL;
struct cleanup *cleanup = make_cleanup (free_current_contents,
                                        &concatenated_name);

... and then just ignore the cleanup chain in the loop.

Keith> +      sprintf (concatenated_name, "%s::%s", base_name, name);

... or maybe some other approach since we're trying to get rid of
sprintf uses.

Otherwise this looks good to me.

Tom