Re: [PATCH 1/2] MIPS: Compressed PLT/stubs support

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Maciej" == Maciej W Rozycki <macro@codesourcery.com> writes:

Maciej> +		 NOTE: macro-20130129: The MIPS backend uses the 8 LSBs of

I prefer not putting usernames and dates into the comments.
That is readily found in the history -- but also more commonly just not
needed.

Maciej>  	      if (type != ST_SYNTHETIC)
Maciej>  		elf_sym = (elf_symbol_type *) sym;
Maciej> -	      else
Maciej> +	      else if (((sym->udata.i | 0xff) ^ 0xff) != 0)
Maciej>  		elf_sym = (elf_symbol_type *) sym->udata.p;
Maciej> +	      else
Maciej> +		elf_sym = NULL;

This seems iffy to me.  I suppose what gives me pause is the possibility
that this condition will trigger on some other port that does something
odd in BFD.

I'm not really an expert here -- I'd appreciate other comments.

Maciej> -/* In SVR4, we recognize a trampoline by it's section name. 
Maciej> -   That is, if the pc is in a section named ".plt" then we are in
Maciej> -   a trampoline.  */
Maciej> +/* In SVR4, we recognize a trampoline by it's section name.  That is,
Maciej> +   if the pc is in a section named ".plt" then we are in a trampoline.
Maciej> +   We let targets request an alternative name, this is currently used
Maciej> +   by the MIPS backend to handle the SVR4 lazy resolution stubs that
Maciej> +   binutils put into ".MIPS.stubs" instead.  */

IMO comments like this tend to get stale over time.
It is better, I think, to describe the function's interface and purpose,
and leave it to future developers to grep for uses of it, should they
need to know.

Tom