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

Sat Oct 7 14:57:45 GMT 2023

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.

 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