Re: [Qemu-devel] [PATCH 1/4] add byteordered types to qemu.

[Christoph Hellwig]

On Thu, Oct 02, 2008 at 02:46:41PM +0200, Gerd Hoffmann wrote:
> Christoph Hellwig wrote:
> 
> >> +typedef uint16_t le16;
> >> +typedef uint32_t le32;
> >> +typedef uint64_t le64;
> >> +typedef uint16_t be16;
> >> +typedef uint32_t be32;
> >> +typedef uint64_t be64;
> > 
> > Any chance you could add sparse __bitwise__ declarations so one could
> > use spare to check for using these types correctly?
> 
> Is there any good documentation on that?

I don't think so.  But I've recently added support for it in the XFS
userspace tools, so I have some experience.

> 
> Looking at the linux kernel sources I find the bitwise stuff depend on
> the __CHECKER__ and __CHECK_ENDIAN__ defines.  I can't find references
> to these in the sparse man page and sparse FAQ (sparse 0.4.1 as shipped
> by fedora 9).
> 
> I can somehow guess what these two defines do.  __CHECKER__ probably is
> set by sparse, and __CHECK_ENDIAN__ by the -Wbitwise switch.  But I
> can't see any obvious reason why include/linux/types.h defines both
> __bitwise__ and __bitwise.  I'd simply use something along the lines ...

Actually __CHECKER__ is set by sparse.  __CHECK_ENDIAN__ is set on the
make command line when checking for endianess bugs.  For xfsprogs I
defined the bitwise annotations unconditionally because we made sure
to not have any of this warnings left (this was quite easy becaus a lot
of the code came from the kernel and was already properly annotated)

These are the snipplets from xfsprogs that would be useful for qemu:

#ifdef __CHECKER__
#define __bitwise       __attribute__((bitwise))
#define __force         __attribute__((force))
#else
#define __bitwise
#define __force
#endif

The __force is needed for the actual swab macros to turn the __bitwise
annotated type back into the normal so it doesn't warn.  For XFS these
conversion macros looks the same as the kernel ones:

#ifdef XFS_NATIVE_HOST
#define cpu_to_be16(val)       ((__force __be16)(__u16)(val))
#define cpu_to_be32(val)       ((__force __be32)(__u32)(val))
#define cpu_to_be64(val)       ((__force __be64)(__u64)(val))
#define be16_to_cpu(val)       ((__force __u16)(__be16)(val))
#define be32_to_cpu(val)       ((__force __u32)(__be32)(val))
#define be64_to_cpu(val)       ((__force __u64)(__be64)(val))
#else
#define cpu_to_be16(val)       ((__force __be16)__swab16((__u16)(val)))
#define cpu_to_be32(val)       ((__force __be32)__swab32((__u32)(val)))
#define cpu_to_be64(val)       ((__force __be64)__swab64((__u64)(val)))
#define be16_to_cpu(val)       (__swab16((__force __u16)(__be16)(val)))
#define be32_to_cpu(val)       (__swab32((__force __u32)(__be32)(val)))
#define be64_to_cpu(val)       (__swab64((__force __u64)(__be64)(val)))
#endif

and then you define the types as __bitwise:

typedef __u16 __bitwise                __be16;
typedef __u32 __bitwise                __be32;
typedef __u64 __bitwise                __be64;

for qemu you'll need slight changes in the names, and also little
endian variants which we don't have in XFS.

Note that the biggest hurdle for xfsprogs was to convince libtool
that cgcc, the gcc wrapper for invoking sparse actually is a C compiler,
but that should be mood for qemu.

Note that sparse will probably complain about a lot of things in
qemu, so you'll have to add a lot -Wno-foo options in addition to
-Wbitwise.