Re: [Qemu-devel] [PATCH v5 07/76] elf: Remove duplicate preprocessor constant definition

[Peter Maydell]

On 1 August 2018 at 19:51, Aleksandar Markovic <address@hidden> wrote:
> Hi, Laurent.
>
>> From: Laurent Vivier <address@hidden>
>> Sent: Monday, July 30, 2018 6:49 PM
>>
>> Le 30/07/2018 à 18:11, Aleksandar Markovic a écrit :
>> > diff --git a/include/elf.h b/include/elf.h
>> > index 934dbbd..c8aaa2a 100644
>> > --- a/include/elf.h
>> > +++ b/include/elf.h
>> > @@ -33,7 +33,6 @@ typedef int64_t  Elf64_Sxword;
>> >
>> >  /* Flags in the e_flags field of the header */
>> >  /* MIPS architecture level. */
>> > -#define EF_MIPS_ARCH            0xf0000000
>> >
>> >  /* Legal values for MIPS architecture level.  */
>> >  #define EF_MIPS_ARCH_1               0x00000000      /* -mips1 code.  */
>> >
>>
>> You should move the comment "MIPS architecture level" where the other
>> EF_MIPS_ARCH is defined (see glibc or system elf.h).
>>
>> Thanks,
>> Laurent
>
> The comment "/* MIPS architecture level */" was present both
> before and after the commit 45506bdd. The scope of this patch
> is to rollback a duplicate preprocessor definition originating
> from the commit 45506bdd. This means that moving/modifying
> comments should not be in this patch. Can you please clarify
> for me what additional changes you suggest (but these should
> definitely be in a separate patch)? Do you want to sync this
> segment of QEMU's elf.h (copied below) with glibc corresponding
> section from its elf.h (also below)?

It seems to me that the logical place for defining EF_MIPS_ARCH
is next to the definitions EF_MIPS_ARCH_1 &c which give the
valid values for that field. It's likely because the
pre-45506bdd QEMU header didn't put the two together as
you'd expect that the duplicate got added.

We should fix the duplication not by just rolling back to
pre-45506bdd, but in a way that gets us to having the
EF_MIPS_ARCH and EF_MIPS_ARCH_1 &c in the same place in
the file. The simplest way to do that is just to delete
the second (original) EF_MIPS_ARCH #define, the one just
below the EF_MIPS_NAN2008 define.

We could line up with the order the glibc headers use, but
that seems like overkill compared to just deleting a line.

thanks
-- PMM