Re: [PATCH 10/27] Replace config-time define HOST_WORDS_BIGENDIAN

[Thomas Huth]

On 16/03/2022 12.15, Halil Pasic wrote:
> On Wed, 16 Mar 2022 11:28:59 +0100
> Thomas Huth <thuth@redhat.com> wrote:
>
> > On 16/03/2022 10.53, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Replace a config-time define with a compile time condition
> > > define (compatible with clang and gcc) that must be declared prior to
> > > its usage. This avoids having a global configure time define, but also
> > > prevents from bad usage, if the config header wasn't included before.
> > >
> > > This can help to make some code independent from qemu too.
> > >
> > > gcc supports __BYTE_ORDER__ from about 4.6 and clang from 3.2.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > [...]
> > > @@ -188,7 +188,7 @@ CPU_CONVERT(le, 64, uint64_t)
> > >     * a compile-time constant if you pass in a constant.  So this can be
> > >     * used to initialize static variables.
> > >     */
> > > -#if defined(HOST_WORDS_BIGENDIAN)
> > > +#if HOST_BIG_ENDIAN
> > >    # define const_le32(_x)                          \
> > >        ((((_x) & 0x000000ffU) << 24) |              \
> > >         (((_x) & 0x0000ff00U) <<  8) |              \
> > > @@ -211,7 +211,7 @@ typedef union {
> > >    
> > >    typedef union {
> > >        float64 d;
> > > -#if defined(HOST_WORDS_BIGENDIAN)
> > > +#if HOST_BIG_ENDIAN
> > >        struct {
> > >            uint32_t upper;
> > >            uint32_t lower;
> > > @@ -235,7 +235,7 @@ typedef union {
> > >    
> > >    typedef union {
> > >        float128 q;
> > > -#if defined(HOST_WORDS_BIGENDIAN)
> > > +#if HOST_BIG_ENDIAN
> > >        struct {
> > >            uint32_t upmost;
> > >            uint32_t upper;
> > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> > > index 0a5e67fb970e..7fdd88adb368 100644
> > > --- a/include/qemu/compiler.h
> > > +++ b/include/qemu/compiler.h
> > > @@ -7,6 +7,8 @@
> > >    #ifndef COMPILER_H
> > >    #define COMPILER_H
> > >    
> > > +#define HOST_BIG_ENDIAN (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
> >
> > Why don't you do it this way instead:
> >
> > #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> > #define HOST_WORDS_BIGENDIAN 1
> > #endif
> >
> > ... that way you could avoid the churn in all the other files?
>
> I guess "prevents from bad usage, if the config header wasn't included
> before" from the commit message is the answer to that question. I agree
> that it is more robust. If we keep the #if defined we really can't
> differentiate between "not defined because not big-endian" and "not
> defined because the appropriate header was not included."

Ok, fair point, now I got it.

Acked-by: Thomas Huth <thuth@redhat.com>