Re: Parenthesize macros in xfs
From: Jan Engelhardt
Date: Sat Mar 25 2006 - 16:39:46 EST
>> >> while browsing through the xfs/linux source, I noticed that many macros do
>> >> not do proper bracing. I have started to cook up a patch, but would like
>> >> feedback first before I continue for nothing.
>> >> It goes like this:
>> >> ...
>> >
>> >That looks fine. Please be sure to work on the -mm tree or on
>> >CVS on oss.sgi.com, so as to reduce your level of patch conflict.
>>
>> Hm, would not it even be better to make them 'static inline' functions?
>
>Probably, I guess I'd want to see how invasive the patch becomes...?
>I really dislike those _ACL macros (around that example you gave, that
>could do with a cleanup all of its own - switching to xfs_acl_ maybe).
>Also watch for macros that modify their parameters, I got burned by
>doing an inline conversion a few releases back on just such a beast..
>
To implement static inline, it requires that header files be corrected
first so that they could 'compile on their own'. Shall I post a patch for
that too?
Here is the first patch in the series of 'fix most things that annoyed me'.
- - - - - - - - - -
Multiple instruction #defines should be wrapped into do{}while(0).
Signed-off-by: Jan Engelhardt <jengelh@xxxxxx>
Compile-tested.
diff -dpru xfs-cvs/dmapi/xfs_dm.c xfs-mod/dmapi/xfs_dm.c
--- xfs-cvs/dmapi/xfs_dm.c 2006-03-17 15:28:04.000000000 +0100
+++ xfs-mod/dmapi/xfs_dm.c 2006-03-25 21:23:45.035287000 +0100
@@ -52,9 +52,10 @@
#define MAXNAMLEN MAXNAMELEN
-#define XFS_BHV_LOOKUP(vp, xbdp) \
+#define XFS_BHV_LOOKUP(vp, xbdp) do { \
xbdp = vn_bhv_lookup(VN_BHV_HEAD(vp), &xfs_vnodeops); \
- ASSERT(xbdp);
+ ASSERT(xbdp); \
+ } while(0)
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
#define MIN_DIO_SIZE(mp) ((mp)->m_sb.sb_sectsize)
@@ -156,7 +157,7 @@ STATIC dm_size_t dm_min_dio_xfer = 0; /
#define DM_STAT_ALIGN (sizeof(__uint64_t))
/* DMAPI's E2BIG == EA's ERANGE */
-#define DM_EA_XLATE_ERR(err) { if (err == ERANGE) err = E2BIG; }
+#define DM_EA_XLATE_ERR(err) do { if (err == ERANGE) err = E2BIG; } while(0)
/*
* xfs_dm_send_data_event()
diff -dpru xfs-mod-01/support/qsort.c xfs-mod-02/support/qsort.c
--- xfs-mod-01/support/qsort.c 2005-09-23 05:51:28.000000000 +0200
+++ xfs-mod-02/support/qsort.c 2006-03-25 22:20:37.055287000 +0100
@@ -55,13 +55,14 @@ swapfunc(char *a, char *b, int n, int sw
swapcode(char, a, b, n)
}
-#define swap(a, b) \
+#define swap(a, b) do { \
if (swaptype == 0) { \
long t = *(long *)(a); \
*(long *)(a) = *(long *)(b); \
*(long *)(b) = t; \
} else \
- swapfunc(a, b, es, swaptype)
+ swapfunc(a, b, es, swaptype) \
+} while(0)
#define vecswap(a, b, n) if ((n) > 0) swapfunc(a, b, n, swaptype)
diff -dpru xfs-cvs/xfs_arch.h xfs-mod/xfs_arch.h
--- xfs-cvs/xfs_arch.h 2006-01-12 03:43:50.000000000 +0100
+++ xfs-mod/xfs_arch.h 2006-03-25 21:37:04.775287000 +0100
@@ -161,14 +161,14 @@
*/
/* does not return a value */
-#define INT_XLATE(buf,mem,dir,arch) {\
+#define INT_XLATE(buf,mem,dir,arch) do { \
ASSERT(dir); \
if (dir>0) { \
(mem)=INT_GET(buf, arch); \
} else { \
INT_SET(buf, arch, mem); \
} \
-}
+} while(0)
static inline void be16_add(__be16 *a, __s16 b)
{
diff -dpru xfs-cvs/xfs_fs.h xfs-mod/xfs_fs.h
--- xfs-cvs/xfs_fs.h 2005-12-02 03:48:45.000000000 +0100
+++ xfs-mod/xfs_fs.h 2006-03-25 21:37:22.325287000 +0100
@@ -122,12 +122,13 @@ struct getbmapx {
#define BMV_OF_PREALLOC 0x1 /* segment = unwritten pre-allocation */
/* Convert getbmap <-> getbmapx - move fields from p1 to p2. */
-#define GETBMAP_CONVERT(p1,p2) { \
+#define GETBMAP_CONVERT(p1,p2) do { \
p2.bmv_offset = p1.bmv_offset; \
p2.bmv_block = p1.bmv_block; \
p2.bmv_length = p1.bmv_length; \
p2.bmv_count = p1.bmv_count; \
- p2.bmv_entries = p1.bmv_entries; }
+ p2.bmv_entries = p1.bmv_entries; \
+ } while(0)
/*
diff -dpru xfs-cvs/xfs_vfsops.c xfs-mod/xfs_vfsops.c
--- xfs-cvs/xfs_vfsops.c 2006-03-25 21:10:18.925287000 +0100
+++ xfs-mod/xfs_vfsops.c 2006-03-25 21:37:36.765287000 +0100
@@ -949,7 +949,7 @@ xfs_sync_inodes(
* must be locked when this is called. After the call the list will no
* longer be locked.
*/
-#define IPOINTER_INSERT(ip, mp) { \
+#define IPOINTER_INSERT(ip, mp) do { \
ASSERT(ipointer_in == B_FALSE); \
ipointer->ip_mnext = ip->i_mnext; \
ipointer->ip_mprev = ip; \
@@ -959,14 +959,14 @@ xfs_sync_inodes(
XFS_MOUNT_IUNLOCK(mp); \
mount_locked = B_FALSE; \
IPOINTER_SET; \
- }
+ } while(0)
/* Remove the marker from the inode list. If the marker was the only item
* in the list then there are no remaining inodes and we should zero out
* the whole list. If we are the current head of the list then move the head
* past us.
*/
-#define IPOINTER_REMOVE(ip, mp) { \
+#define IPOINTER_REMOVE(ip, mp) do { \
ASSERT(ipointer_in == B_TRUE); \
if (ipointer->ip_mnext != (xfs_inode_t *)ipointer) { \
ip = ipointer->ip_mnext; \
@@ -981,7 +981,7 @@ xfs_sync_inodes(
ip = NULL; \
} \
IPOINTER_CLR; \
- }
+ } while(0)
#define XFS_PREEMPT_MASK 0x7f
diff -dpru xfs-cvs/xfs_vnodeops.c xfs-mod/xfs_vnodeops.c
--- xfs-cvs/xfs_vnodeops.c 2006-03-25 21:10:19.085287000 +0100
+++ xfs-mod/xfs_vnodeops.c 2006-03-25 21:37:48.405287000 +0100
@@ -2307,7 +2307,7 @@ again:
}
#ifdef DEBUG
-#define REMOVE_DEBUG_TRACE(x) {remove_which_error_return = (x);}
+#define REMOVE_DEBUG_TRACE(x) do { remove_which_error_return = (x); } while(0)
int remove_which_error_return = 0;
#else /* ! DEBUG */
#define REMOVE_DEBUG_TRACE(x)
diff -dpru xfs-cvs/xfsidbg.c xfs-mod/xfsidbg.c
--- xfs-cvs/xfsidbg.c 2006-03-25 21:10:19.105287000 +0100
+++ xfs-mod/xfsidbg.c 2006-03-25 21:37:58.325287000 +0100
@@ -7354,7 +7354,7 @@ xfsidbg_xqm_dquot(xfs_dquot_t *dqp)
#define XQMIDBG_LIST_PRINT(l, NXT) \
-{ \
+do { \
xfs_dquot_t *dqp;\
int i = 0; \
kdb_printf("[#%d dquots]\n", (int) (l)->qh_nelems); \
@@ -7367,7 +7367,7 @@ xfsidbg_xqm_dquot(xfs_dquot_t *dqp)
(int) be64_to_cpu(dqp->q_core.d_icount), \
(int) dqp->q_nrefs); }\
kdb_printf("\n"); \
-}
+} while(0)
static void
xfsidbg_xqm_dqattached_inos(xfs_mount_t *mp)
#<<eof>>
Jan Engelhardt
--
| Software Engineer and Linux/Unix Network Administrator
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/