Re: Parenthesize macros in xfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>> >> 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 <[email protected]>
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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux