Re: [PATCH] MIPS: fix mips16 symbol identification

["Maciej W. Rozycki" <macro at codesourcery dot com>]

Hi Shun-Yen,

> A mips16 symbol may contains non-zero visibility bits, so original
> implementation
> cannot correctly identify mips16 internal/hidden/protected symbols.
> 
> The fix below uses ELF_ST_IS_MIPS16 macro to check mips16 symbol instead of
> checking st_other directly.

 Thank you for your contribution.  Interestingly enough I have a similar 
change included with a larger set that I'll be pushing in the near future.  
Until then your change is of course perfectly valid, however it has a 
couple of small problems, as noted below.

> 2012-04-18  Shun-Yen Lu  <dark.asparagus@gmail.com>
> 
> 	gdb/
> 	* mips-tdep.c (mips_elf_make_msymbol_special): Fix identification
> 	of mips16 symbol.

 s/symbol/symbols/ -- I think that would sound (read) better.

> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -268,7 +268,7 @@ mips_abi_regsize (struct gdbarch *gdbarch)
> ?static void
> ?mips_elf_make_msymbol_special (asymbol * sym, struct minimal_symbol *msym)
> ?{
> -? if (((elf_symbol_type *) (sym))->internal_elf_sym.st_other == STO_MIPS16)
> +? if (ELF_ST_IS_MIPS16(((elf_symbol_type *) (sym))->internal_elf_sym.st_other))
> ???? {
> ?????? MSYMBOL_TARGET_FLAG_1 (msym) = 1;
> ???? }

 You need to follow the GNU coding standard, there has to be a space 
between ELF_ST_IS_MIPS16 and the following opening bracked (just as with 
MSYMBOL_TARGET_FLAG_1 below), and then you'll have to wrap the line as it 
won't fit in 79 columns anymore (keeping the indentation of the 
continuation line correct).  Please resubmit with these changes made.

 I've noticed you are not listed in the MAINTAINERS file, do you have a 
copyright assignment in place with FSF and repository write access?  If 
not, then I am going to accept this change as trivial enough not to 
require the assignment, however if you plan to make further contributions, 
then you'll need to make such an assignment.  I can give you detailed 
instructions if interested.

  Maciej