Message ID | 1424196067.27855.80.camel@ubuntu-sellcey |
---|---|
State | New |
Headers | show |
On Tue, 17 Feb 2015, Steve Ellcey wrote: > You are right. Apparently, this header is not actually used in the > glibc build since it didn't cause a build failure but we should fix it There are both sysdeps/mips/bits/endian.h (not used by any current configuration) and sysdeps/unix/sysv/linux/mips/bits/endian.h - clearly not both needed since one bits/endian.h file could be generic. They should be unified. (I'd say prefer sysdeps/mips/bits/endian.h as symmetric between the endiannesses, but with the comment corrected as "This file is for a machine using big-endian mode." is simply wrong.)
On Tue, 17 Feb 2015, Joseph Myers wrote: > > You are right. Apparently, this header is not actually used in the > > glibc build since it didn't cause a build failure but we should fix it > > There are both sysdeps/mips/bits/endian.h (not used by any current > configuration) and sysdeps/unix/sysv/linux/mips/bits/endian.h - clearly > not both needed since one bits/endian.h file could be generic. They > should be unified. (I'd say prefer sysdeps/mips/bits/endian.h as > symmetric between the endiannesses, but with the comment corrected as > "This file is for a machine using big-endian mode." is simply wrong.) There's clearly nothing Linux specific there although one might ask which of the two of __MIPSEB vs __MIPSEB__ is the "traditional" macro we might want to keep. Or maybe _MIPSEB even. I'm not sure offhand and clearly current GCC defines all at a time. Also I wonder if sanity checks like: #if defined (__MIPSEB) && defined (__MIPSEL) # error "Both __MIPSEB and __MIPSEL, endianness configuration problem?" #elif defined (__MIPSEB) # define __BYTE_ORDER __BIG_ENDIAN #elif defined (__MIPSEL) # define __BYTE_ORDER __LITTLE_ENDIAN #elif # error "Neither __MIPSEB nor __MIPSEL, endianness configuration problem?" #endif would make sense here too, to catch issues early on in case someone does something silly with `-D' or `-U'. Maciej
On Tue, 2015-02-17 at 19:58 +0000, Maciej W. Rozycki wrote: > There's clearly nothing Linux specific there although one might ask which > of the two of __MIPSEB vs __MIPSEB__ is the "traditional" macro we might > want to keep. Or maybe _MIPSEB even. I'm not sure offhand and clearly > current GCC defines all at a time. > > Also I wonder if sanity checks like: > > #if defined (__MIPSEB) && defined (__MIPSEL) > # error "Both __MIPSEB and __MIPSEL, endianness configuration problem?" > #elif defined (__MIPSEB) > # define __BYTE_ORDER __BIG_ENDIAN > #elif defined (__MIPSEL) > # define __BYTE_ORDER __LITTLE_ENDIAN > #elif > # error "Neither __MIPSEB nor __MIPSEL, endianness configuration problem?" > #endif > > would make sense here too, to catch issues early on in case someone does > something silly with `-D' or `-U'. > > Maciej I wonder if we should check here or if we should check in the shared src/glibc/string/endian.h file. We could verify that after including a system specific bits/endian.h, __BYTE_ORDER is defined to either __BIG_ENDIAN or __LITTLE_ENDIAN. Maybe change this check in src/glibc/string/endian.h to have a '#else #error' clause. Is it ever legal to not define __BYTE_ORDER? #if __BYTE_ORDER == __LITTLE_ENDIAN # define __LONG_LONG_PAIR(HI, LO) LO, HI #elif __BYTE_ORDER == __BIG_ENDIAN # define __LONG_LONG_PAIR(HI, LO) HI, LO #endif Steve Ellcey
The purpose of <bits/endian.h> is to define __BYTE_ORDER, so a sanity check that it indeed does so is certainly fine.
diff --git a/sysdeps/mips/bits/endian.h b/sysdeps/mips/bits/endian.h index 43ce009..92e97c7 100644 --- a/sysdeps/mips/bits/endian.h +++ b/sysdeps/mips/bits/endian.h @@ -8,6 +8,6 @@ #ifdef __MIPSEB # define __BYTE_ORDER __BIG_ENDIAN #endif -#if __MIPSEL +#ifdef __MIPSEL # define __BYTE_ORDER __LITTLE_ENDIAN #endif