On 1 August 2018 at 19:51, Aleksandar Markovic <amarko...@wavecomp.com> wrote: > Hi, Laurent. > >> From: Laurent Vivier <laur...@vivier.eu> >> 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