This is the mail archive of the
mailing list for the GDB project.
Re: [patch] Change cplus_specific to an alocated struct
- From: Tom Tromey <tromey at redhat dot com>
- To: sami wagiaalla <swagiaal at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 08 Jun 2010 11:02:15 -0600
- Subject: Re: [patch] Change cplus_specific to an alocated struct
- References: <4BFD4230.firstname.lastname@example.org>
- Reply-to: tromey at redhat dot com
>>>>> "Sami" == sami wagiaalla <email@example.com> writes:
Sami> I plan to expand the cplus_specific struct to store template
Sami> information needed, of course, only for C++. Tom suggested that I move
Sami> the substruct out of struct general_symbol_info and change its
Sami> reference to a pointer to be assigned to the struct if allocated.
Sami> Thoughts ?
I think the idea is fine.
Sami> char *
Sami> ada_decode_symbol (const struct general_symbol_info *gsymbol)
Sami> - char **resultp =
Sami> - (char **) &gsymbol->language_specific.cplus_specific.demangled_name;
Sami> - if (*resultp == NULL)
Sami> + char *result = symbol_get_cplus_demangled_name (gsymbol);
I don't think this change is correct -- but FWIW I don't think the
existing code is correct, either.
The code is written this way because this function caches the demangled
name in the cplus_specific struct. Your change removes this caching.
However, this takes a general_symbol_info and so, presumably, can be
used for partial symbols. But, modifying a partial symbol is a no-no,
because they are stored in a bcache.
All this reminds me -- you should look at bcache utilization and memory
use before and after your changes to make sure we aren't hitting a
memory use regression here. There is some command that will show you
bcache statistics, though I forget what it is offhand.
Sami> +char *
Sami> +symbol_get_cplus_demangled_name (const struct general_symbol_info *gsymbol)
Sami> + if (gsymbol->language_specific.cplus_specific != NULL)
Sami> + return gsymbol->language_specific.cplus_specific->demangled_name;
Sami> + return NULL;
Perhaps we should have different types of structs in the
language_specific union, and have functions like this examine the
It seems to me that soon we're going to want to add a bunch of
C++-specific fields, and we don't want to unnecessarily penalize the
other languages with our baggage.
If you were planning that for a follow-on patch, that is fine -- but
also the sort of thing that it is handy to note in your submissions.
Sami> +/* Initialize the cplus_specific structure. 'cplus_specific' is intended to
Sami> + be allocated lazily. So this should only be called if the structure is
Sami> + needed. */
Sami> +static void
Sami> +symbol_init_cplus_specific (struct general_symbol_info *gsymbol,
Sami> + struct objfile *objfile)
Initializing lazily is usually good, but it breaks the bcache.
However perhaps this can be fixed by having the partial symbol code
force the issue.