Re: [PATCH] MIPS: Change all E_MIPS_* to EF_MIPS_*

["Ying Huang" <ying.huang@oss.cipunited.com>]

[-- Attachment #1: Type: text/plain, Size: 8319 bytes --]

Hi Maciej,

在 2023/10/7 22:57, Maciej W. Rozycki 写道:
> On Fri, 18 Aug 2023, Ying Huang wrote:
>
>> From: Ying Huang <huangyin@oss.cipunited.com>
>>
>> The purpose is to keep same with glibc elf.h
>  Just as I wrote in the context of the glibc patch:
>
>>  To move forward you need to put the findings from the discussion in the 
>> change description, especially given the difficulty to figure out what the 
>> correct naming convention for the ELF file header flags is supposed to be.  
>> This is so that the next time this information is needed it is readily 
>> available rather than requiring chasing in the archives and/or depending 
>> on the presence of people who may still remember it as on this occasion.
> and:
>
>>  Then the binutils change needs to document the switch from E_* to EF_* 
>> definitions accordingly.
> Please update the change description as requested.  I can see it wasn't 
> done for glibc and I didn't stop it in time due to other commitments, but 
> failing to do so for glibc is not an excuse for binutils.  Add a reference 
> to David Anderson's message in particular, as it's key.

I had concluded the discussion in glibc patch but may be not complete.

And I would include this reference <https://sourceware.org/ml/binutils/2002-07/msg00681.html> to binutils commit messages.

>
>  There are some issues with your change as listed below, please fix them 
> too.
>
>> diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
>> index 92dd4c20a7d..4b1ec670dc0 100644
>> --- a/bfd/elfxx-mips.c
>> +++ b/bfd/elfxx-mips.c
>> @@ -12319,68 +12319,68 @@ mips_set_isa_flags (bfd *abfd)
>>      {
>>      default:
>>        if (ABI_N32_P (abfd) || ABI_64_P (abfd))
>> -        val = MIPS_DEFAULT_R6 ? E_MIPS_ARCH_64R6 : E_MIPS_ARCH_3;
>> +        val = MIPS_DEFAULT_R6 ? EF_MIPS_ARCH_64R6 : EF_MIPS_ARCH_3;
>>        else
>> -        val = MIPS_DEFAULT_R6 ? E_MIPS_ARCH_32R6 : E_MIPS_ARCH_1;
>> +        val = MIPS_DEFAULT_R6 ? EF_MIPS_ARCH_32R6 : EF_MIPS_ARCH_1;
>  Use tabs rather than spaces to indent these lines.
>
>> diff --git a/elfcpp/mips.h b/elfcpp/mips.h
>> index daf3f5472f4..bc95277df50 100644
>> --- a/elfcpp/mips.h
>> +++ b/elfcpp/mips.h
>> @@ -493,8 +493,8 @@ abi_n32(elfcpp::Elf_Word e_flags)
>>  bool
>>  r6_isa(elfcpp::Elf_Word e_flags)
>>  {
>> -  return ((e_flags & elfcpp::EF_MIPS_ARCH) == elfcpp::E_MIPS_ARCH_32R6)
>> -           || ((e_flags & elfcpp::EF_MIPS_ARCH) == elfcpp::E_MIPS_ARCH_64R6);
>> +  return ((e_flags & elfcpp::EF_MIPS_ARCH) == elfcpp::EF_MIPS_ARCH_32R6)
>> +           || ((e_flags & elfcpp::EF_MIPS_ARCH) == elfcpp::EF_MIPS_ARCH_64R6);
>  Bad indentation here, `||' doesn't align with the correct level of 
> parentheses above.
>
>> diff --git a/gas/config.in b/gas/config.in
>> index 232bc350759..ea21757f6a7 100644
>> --- a/gas/config.in
>> +++ b/gas/config.in
>> @@ -264,8 +264,8 @@
>>  /* Use emulation support? */
>>  #undef USE_EMULATIONS
>>  
>> -/* Allow use of E_MIPS_ABI_O32 on MIPS targets. */
>> -#undef USE_E_MIPS_ABI_O32
>> +/* Allow use of EF_MIPS_ABI_O32 on MIPS targets. */
>> +#undef USE_EF_MIPS_ABI_O32
>>  
>>  /* Enable extensions on AIX 3, Interix.  */
>>  #ifndef _ALL_SOURCE
>  Please regenerate this file correctly as I can see differences if I do it 
> locally with your change applied.
>
>> diff --git a/include/elf/mips.h b/include/elf/mips.h
>> index 2c13cc88b40..48d54a1808f 100644
>> --- a/include/elf/mips.h
>> +++ b/include/elf/mips.h
>> @@ -223,52 +223,52 @@ END_RELOC_NUMBERS (R_MIPS_maxext)
>>  #define EF_MIPS_ARCH		0xf0000000
>>  
>>  /* -mips1 code.  */
>> -#define E_MIPS_ARCH_1		0x00000000
>> +#define EF_MIPS_ARCH_1		0x00000000
>>  
>>  /* -mips2 code.  */
>> -#define E_MIPS_ARCH_2		0x10000000
>> +#define EF_MIPS_ARCH_2		0x10000000
>>  
>>  /* -mips3 code.  */
>> -#define E_MIPS_ARCH_3		0x20000000
>> +#define EF_MIPS_ARCH_3		0x20000000
>>  
>>  /* -mips4 code.  */
>> -#define E_MIPS_ARCH_4		0x30000000
>> +#define EF_MIPS_ARCH_4		0x30000000
>>  
>>  /* -mips5 code.  */
>> -#define E_MIPS_ARCH_5           0x40000000
>> +#define EF_MIPS_ARCH_5           0x40000000
>>  
>>  /* -mips32 code.  */
>> -#define E_MIPS_ARCH_32          0x50000000
>> +#define EF_MIPS_ARCH_32          0x50000000
>>  
>>  /* -mips64 code.  */
>> -#define E_MIPS_ARCH_64          0x60000000
>> +#define EF_MIPS_ARCH_64          0x60000000
>>  
>>  /* -mips32r2 code.  */
>> -#define E_MIPS_ARCH_32R2        0x70000000
>> +#define EF_MIPS_ARCH_32R2        0x70000000
>>  
>>  /* -mips64r2 code.  */
>> -#define E_MIPS_ARCH_64R2        0x80000000
>> +#define EF_MIPS_ARCH_64R2        0x80000000
>>  
>>  /* -mips32r6 code.  */
>> -#define E_MIPS_ARCH_32R6        0x90000000
>> +#define EF_MIPS_ARCH_32R6        0x90000000
>>  
>>  /* -mips64r6 code.  */
>> -#define E_MIPS_ARCH_64R6        0xa0000000
>> +#define EF_MIPS_ARCH_64R6        0xa0000000
>>  
>>  /* The ABI of the file.  Also see EF_MIPS_ABI2 above. */
>>  #define EF_MIPS_ABI		0x0000F000
>>  
>>  /* The original o32 abi. */
>> -#define E_MIPS_ABI_O32          0x00001000
>> +#define EF_MIPS_ABI_O32          0x00001000
>>  
>>  /* O32 extended to work on 64 bit architectures */
>> -#define E_MIPS_ABI_O64          0x00002000
>> +#define EF_MIPS_ABI_O64          0x00002000
>>  
>>  /* EABI in 32 bit mode */
>> -#define E_MIPS_ABI_EABI32       0x00003000
>> +#define EF_MIPS_ABI_EABI32       0x00003000
>>  
>>  /* EABI in 64 bit mode */
>> -#define E_MIPS_ABI_EABI64       0x00004000
>> +#define EF_MIPS_ABI_EABI64       0x00004000
>  Inconsistent formatting here, use tabs to separate names from values.
>
>> @@ -281,28 +281,28 @@ END_RELOC_NUMBERS (R_MIPS_maxext)
>>     00 - 7F should be left for a future standard;
>>     the rest are open. */
>>  
>> -#define E_MIPS_MACH_3900	0x00810000
>> -#define E_MIPS_MACH_4010	0x00820000
>> -#define E_MIPS_MACH_4100	0x00830000
>> -#define E_MIPS_MACH_ALLEGREX	0x00840000
>> -#define E_MIPS_MACH_4650	0x00850000
>> -#define E_MIPS_MACH_4120	0x00870000
>> -#define E_MIPS_MACH_4111	0x00880000
>> -#define E_MIPS_MACH_SB1         0x008a0000
>> -#define E_MIPS_MACH_OCTEON	0x008b0000
>> -#define E_MIPS_MACH_XLR     	0x008c0000
>> -#define E_MIPS_MACH_OCTEON2	0x008d0000
>> -#define E_MIPS_MACH_OCTEON3	0x008e0000
>> -#define E_MIPS_MACH_5400	0x00910000
>> -#define E_MIPS_MACH_5900	0x00920000
>> -#define E_MIPS_MACH_IAMR2	0x00930000
>> -#define E_MIPS_MACH_5500	0x00980000
>> -#define E_MIPS_MACH_9000	0x00990000
>> -#define E_MIPS_MACH_LS2E        0x00A00000
>> -#define E_MIPS_MACH_LS2F        0x00A10000
>> -#define E_MIPS_MACH_GS464       0x00A20000
>> -#define E_MIPS_MACH_GS464E	0x00A30000
>> -#define E_MIPS_MACH_GS264E	0x00A40000
>> +#define EF_MIPS_MACH_3900	0x00810000
>> +#define EF_MIPS_MACH_4010	0x00820000
>> +#define EF_MIPS_MACH_4100	0x00830000
>> +#define EF_MIPS_MACH_ALLEGREX	0x00840000
>> +#define EF_MIPS_MACH_4650	0x00850000
>> +#define EF_MIPS_MACH_4120	0x00870000
>> +#define EF_MIPS_MACH_4111	0x00880000
>> +#define EF_MIPS_MACH_SB1         0x008a0000
>> +#define EF_MIPS_MACH_OCTEON	0x008b0000
>> +#define EF_MIPS_MACH_XLR     	0x008c0000
>> +#define EF_MIPS_MACH_OCTEON2	0x008d0000
>> +#define EF_MIPS_MACH_OCTEON3	0x008e0000
>> +#define EF_MIPS_MACH_5400	0x00910000
>> +#define EF_MIPS_MACH_5900	0x00920000
>> +#define EF_MIPS_MACH_IAMR2	0x00930000
>> +#define EF_MIPS_MACH_5500	0x00980000
>> +#define EF_MIPS_MACH_9000	0x00990000
>> +#define EF_MIPS_MACH_LS2E        0x00A00000
>> +#define EF_MIPS_MACH_LS2F        0x00A10000
>> +#define EF_MIPS_MACH_GS464       0x00A20000
>> +#define EF_MIPS_MACH_GS464E	0x00A30000
>> +#define EF_MIPS_MACH_GS264E	0x00A40000
>  Likewise.
>
>  In the future please verify your changes for such obvious code formatting 
> issues before submitting as refraining from doing so only makes the review 
> take longer.  Also double-check I haven't missed anything on this occasion 
> too.
>
>  And always use the relevant tools to update generated files, never edit 
> them by hand.
>
>   Maciej

I only modified E_ to EF_, did not do any other actions but lead to these format issues. 

I would find the reason and double check.

Thanks,

Ying