[PATCH] abuse of macros in swab.h

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


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 : Sat Sep 23 2000 - 21:00:21 EST