Re: [PATCH] MIPS: fix mips16 symbol identification

[Shun-Yen Lu <dark dot asparagus at gmail dot com>]

Hi Maciej,

thanks for your response, I didn't fill the copy assignment yet.
Please consider this patch a small change at this time.

So if I would like to join the development of GNU software, I need
to register a GNU account at Savannah and signed & return a copyright
assignment? I am not very clear about this part.


The updated ChangeLog and patch (attachment mips16.symbol.patch):


2012-04-20  Shun-Yen Lu  <dark.asparagus@gmail.com>

      gdb/
      * mips-tdep.c (mips_elf_make_msymbol_special): Fix identification
      of mips16 symbols.


Shun-Yen

å 2012å4æ19æäå8:15ïMaciej W. Rozycki <macro@codesourcery.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

Attachment: mips16.symbol.patch
Description: Binary data