[PATCH] abuse of macros in swab.h

From: Alexander Viro (viro@math.psu.edu)
Date: Tue Sep 19 2000 - 19:13:31 EDT

  • Next message: Andi Kleen: "Re: [PATCH] abuse of macros in swab.h"

    On Tue, 19 Sep 2000, David S. Miller wrote:

    > Date: Wed, 20 Sep 2000 00:50:06 +0200
    > From: "Andi Kleen" <ak@suse.de>
    >
    > This patch fixes an obvious bug introduced with the ext2 changes in
    > 2.2.18pre (look up the definition of le32_to_cpu on BE machines
    > without a special assembler version for it and on machines that
    > have it)
    >
    > --- linux-work/fs/ext2/inode.c-EXT2 Fri Sep 15 02:06:16 2000
    > +++ linux-work/fs/ext2/inode.c Wed Sep 20 00:47:36 2000
    >
    >
    > Durr, this explains why small bits my home directory disk got
    > corrupted while running 2.2.18pre, nice spotting Andi :-)

    Nice spotting, but bad fix, IMO. swab...() stuff is a perfect example of
    the dangerous use of macros. BTW, 2.4 has the same problem.

    How about
    --- include/linux/byteorder/swab.h Tue Sep 19 22:23:43 2000
    +++ include/linux/byteorder/swab.h.new Tue Sep 19 22:29:07 2000
    @@ -13,31 +13,35 @@
      * See asm-i386/byteorder.h and suches for examples of how to provide
      * architecture-dependent optimized versions
      *
    + * They shouldn't be macros, damnit. AV, 20000919
    + *
      */
     
     /* casts are necessary for constants, because we never know how for sure
      * how U/UL/ULL map to __u16, __u32, __u64. At least not in a portable way.
      */
    -#define ___swab16(x) \
    - ((__u16)( \
    - (((__u16)(x) & (__u16)0x00ffU) << 8) | \
    - (((__u16)(x) & (__u16)0xff00U) >> 8) ))
    -#define ___swab32(x) \
    - ((__u32)( \
    - (((__u32)(x) & (__u32)0x000000ffUL) << 24) | \
    - (((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
    - (((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
    - (((__u32)(x) & (__u32)0xff000000UL) >> 24) ))
    -#define ___swab64(x) \
    - ((__u64)( \
    - (__u64)(((__u64)(x) & (__u64)0x00000000000000ffULL) << 56) | \
    - (__u64)(((__u64)(x) & (__u64)0x000000000000ff00ULL) << 40) | \
    - (__u64)(((__u64)(x) & (__u64)0x0000000000ff0000ULL) << 24) | \
    - (__u64)(((__u64)(x) & (__u64)0x00000000ff000000ULL) << 8) | \
    - (__u64)(((__u64)(x) & (__u64)0x000000ff00000000ULL) >> 8) | \
    - (__u64)(((__u64)(x) & (__u64)0x0000ff0000000000ULL) >> 24) | \
    - (__u64)(((__u64)(x) & (__u64)0x00ff000000000000ULL) >> 40) | \
    - (__u64)(((__u64)(x) & (__u64)0xff00000000000000ULL) >> 56) ))
    +static inline __u16 ___swab16(__u16 x)
    +{
    + return ((x & (__u16)0x00ffU) << 8) | ((x & (__u16)0xff00U) >> 8);
    +}
    +static inline __u32 ___swab16(__u32 x)
    +{
    + return ((x & (__u32)0x000000ffUL) << 24) |
    + ((x & (__u32)0x0000ff00UL) << 8) |
    + ((x & (__u32)0x00ff0000UL) >> 8) |
    + ((x & (__u32)0xff000000UL) >> 24);
    +}
    +static inline __u64 ___swab16(__u64 x)
    +{
    + return (__u64)((x & (__u64)0x00000000000000ffULL) << 56) |
    + (__u64)((x & (__u64)0x000000000000ff00ULL) << 40) |
    + (__u64)((x & (__u64)0x0000000000ff0000ULL) << 24) |
    + (__u64)((x & (__u64)0x00000000ff000000ULL) << 8) |
    + (__u64)((x & (__u64)0x000000ff00000000ULL) >> 8) |
    + (__u64)((x & (__u64)0x0000ff0000000000ULL) >> 24) |
    + (__u64)((x & (__u64)0x00ff000000000000ULL) >> 40) |
    + (__u64)((x & (__u64)0xff00000000000000ULL) >> 56);
    +}
     
     /*
      * provide defaults when no architecture-specific optimization is detected

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    Please read the FAQ at http://www.tux.org/lkml/



    This archive was generated by hypermail 2b29 : Tue Sep 19 2000 - 19:25:02 EDT