Re: [patch] Do not add partial_symbol again and again to the list

[Daniel Jacobowitz <drow at false dot org>]

On Mon, May 05, 2008 at 02:58:53PM -0400, Aleksandar Ristovski wrote:
>>> +const void *
>>> +bcache (const void *addr, int length, struct bcache *bcache)
>>> +{
>>> +  return bcache_data (addr, length, bcache);
>>> +}
>>> +
>>> +void *
>>> +bcache_added (const void *addr, int length, struct bcache *bcache,  
>>> +		   int *added)
>>
>> It should return a const pointer, like bcache.  Also the indentation
>> on the second line is too deep.
>
> bcache_added is more like bcache_data which returns void*. It would make 
> sense to return void const* but then I would have to change const-ness in 
> many places (too many: I would leave that for some other patch).

This really is not OK.  Take a look at the explanation of a bcache:

   A bcache is a data structure for factoring out duplication in
   read-only structures.

If you modify the pointed-to object then things go wrong: we put
duplicates in the cache which is what it's supposed to avoid.  And you
modify the other copies which share the same bcache entry, corrupting
them.

If you don't want to fix the constification issues right now, then
just call this deprecated_bcache_added and we can fix it later; you're
not making it any worse.  But this is something you have to understand
about the bcache.

>> In fact, if lookup_partial_symbol will never find a global symbol
>> with an identical name why do we need them for any duplicated symbol,
>> even legitimately duplicated things like static functions?  
>
> I don't think static functions are a good example since they would not go 
> into the global list anyway (see dwarf2read.c, case DW_TAG_subprogram).
>
> The attached is revised patch with the changes above; additionally, it  
> calls new add_partial_symbol_to_global_list for all partial symbols that  
> are being added to the global list. 

This is not symbol-reader specific, right?  There's nothing that makes
it safe for DWARF2 but unsafe for stabs?

If so, a smaller patch would be do just do this in add_psymbol_to_list
if list == &objfile->global_symbols.

-- 
Daniel Jacobowitz
CodeSourcery