From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from angie.orcam.me.uk (angie.orcam.me.uk [78.133.224.34]) by sourceware.org (Postfix) with ESMTP id 4B4173858D37 for ; Sat, 7 Oct 2023 14:57:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4B4173858D37 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orcam.me.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=orcam.me.uk Received: by angie.orcam.me.uk (Postfix, from userid 500) id 7D2E692009C; Sat, 7 Oct 2023 16:57:45 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 7B94592009B; Sat, 7 Oct 2023 15:57:45 +0100 (BST) Date: Sat, 7 Oct 2023 15:57:45 +0100 (BST) From: "Maciej W. Rozycki" To: Ying Huang cc: binutils@sourceware.org, yunqiang.su@oss.cipunited.com, Ying Huang Subject: Re: [PATCH] MIPS: Change all E_MIPS_* to EF_MIPS_* In-Reply-To: <20230818095053.2340246-1-ying.huang@oss.cipunited.com> Message-ID: References: <20230818095053.2340246-1-ying.huang@oss.cipunited.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-1169.3 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_INFOUSMEBIZ,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, 18 Aug 2023, Ying Huang wrote: > From: Ying Huang > > 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