diff mbox

[2/3] cifs: Make CIFS_DEBUG possible to undefine

Message ID 26f6f4e1a1fb5091784fb1a6919e214994a6dc88.1354217746.git.joe@perches.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joe Perches Nov. 29, 2012, 7:37 p.m. UTC
Make the compilation work again when CIFS_DEBUG is not #define'd.

Add format and argument verification for the various macros when
CIFS_DEBUG is not #define'd.

Signed-off-by: Joe Perches <joe@perches.com>
---
 fs/cifs/cifs_debug.h |   64 ++++++++++++++++++++++++++++++++------------------
 1 files changed, 41 insertions(+), 23 deletions(-)

Comments

Jeff Layton Nov. 30, 2012, 11:49 a.m. UTC | #1
On Thu, 29 Nov 2012 11:37:19 -0800
Joe Perches <joe@perches.com> wrote:

> Make the compilation work again when CIFS_DEBUG is not #define'd.
> 
> Add format and argument verification for the various macros when
> CIFS_DEBUG is not #define'd.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  fs/cifs/cifs_debug.h |   64 ++++++++++++++++++++++++++++++++------------------
>  1 files changed, 41 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h
> index b0fc344..4d12fe4 100644
> --- a/fs/cifs/cifs_debug.h
> +++ b/fs/cifs/cifs_debug.h
> @@ -37,6 +37,9 @@ void dump_smb(void *, int);
>  #define CIFS_RC		0x02
>  #define CIFS_TIMER	0x04
>  
> +extern int cifsFYI;
> +extern int cifsERROR;
> +
>  /*
>   *	debug ON
>   *	--------
> @@ -44,36 +47,33 @@ void dump_smb(void *, int);
>  #ifdef CIFS_DEBUG
>  
>  /* information message: e.g., configuration, major event */
> -extern int cifsFYI;
> -#define cifsfyi(fmt, arg...)						\
> +#define cifsfyi(fmt, ...)						\
>  do {									\
>  	if (cifsFYI & CIFS_INFO)					\
> -		printk(KERN_DEBUG "%s: " fmt "\n", __FILE__, ##arg);	\
> +		printk(KERN_DEBUG "%s: " fmt "\n",			\
> +		       __FILE__, ##__VA_ARGS__);			\
>  } while (0)
>  
> -#define cFYI(set, fmt, arg...)			\
> -do {						\
> -	if (set)				\
> -		cifsfyi(fmt, ##arg);		\
> +#define cFYI(set, fmt, ...)						\
> +do {									\
> +	if (set)							\
> +		cifsfyi(fmt, ##__VA_ARGS__);				\
>  } while (0)
>  
> -#define cifswarn(fmt, arg...)			\
> -	printk(KERN_WARNING fmt "\n", ##arg)
> -
> -/* debug event message: */
> -extern int cifsERROR;
> +#define cifswarn(fmt, ...)						\
> +	printk(KERN_WARNING fmt "\n", ##__VA_ARGS__)
>  
>  /* error event message: e.g., i/o error */
> -#define cifserror(fmt, arg...)					\
> -do {								\
> -	if (cifsERROR)						\
> -		printk(KERN_ERR "CIFS VFS: " fmt "\n", ##arg);	\
> +#define cifserror(fmt, ...)						\
> +do {									\
> +	if (cifsERROR)							\
> +		printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__);	\
>  } while (0)
>  
> -#define cERROR(set, fmt, arg...)		\
> -do {						\
> -	if (set)				\
> -		cifserror(fmt, ##arg);		\
> +#define cERROR(set, fmt, ...)						\
> +do {									\
> +	if (set)							\
> +		cifserror(fmt, ##__VA_ARGS__);				\
>  } while (0)
>  
>  /*
> @@ -81,9 +81,27 @@ do {						\
>   *	---------
>   */
>  #else		/* _CIFS_DEBUG */
> -#define cERROR(set, fmt, arg...)
> -#define cFYI(set, fmt, arg...)
> -#define cifserror(fmt, arg...)
> +#define cifsfyi(fmt, ...)						\
> +do {									\
> +	if (0)								\
> +		printk(KERN_DEBUG "%s: " fmt "\n",			\
> +		       __FILE__, ##__VA_ARGS__);			\
> +} while (0)
> +#define cFYI(set, fmt, ...)						\
> +do {									\
> +	if (0 && set)							\
> +		cifsfyi(fmt, ##__VA_ARGS__);				\
> +} while (0)
> +#define cifserror(fmt, ...)						\
> +do {									\
> +	if (0)								\
> +		printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__);	\
> +} while (0)
> +#define cERROR(set, fmt, ...)						\
> +do {									\
> +	if (0 && set)							\
> +		cifserror(fmt, ##__VA_ARGS__);				\
> +} while (0)

Would it be better to simply make those the standard

"do { ; } while(0)" noop macros?

I'm not sure I see the point in keeping the printk statements in there...

>  #endif		/* _CIFS_DEBUG */
>  
>  #endif				/* _H_CIFS_DEBUG */
Joe Perches Nov. 30, 2012, 2:52 p.m. UTC | #2
On Fri, 2012-11-30 at 06:49 -0500, Jeff Layton wrote:
> On Thu, 29 Nov 2012 11:37:19 -0800
> Joe Perches <joe@perches.com> wrote:
> 
> > Make the compilation work again when CIFS_DEBUG is not #define'd.
> > 
> > Add format and argument verification for the various macros when
> > CIFS_DEBUG is not #define'd.
[]
> Would it be better to simply make those the standard
> 
> "do { ; } while(0)" noop macros?
> 
> I'm not sure I see the point in keeping the printk statements in there...

Actually, that's not standard.
Look at printk.h

It prevents developers from adding code that compiles
without warnings in one mode but compiles with warnings
in another.

The idea is to make sure that format and arguments always
match regardless of whether or not you are compiling
debug or non-debug.


--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Layton Nov. 30, 2012, 3:03 p.m. UTC | #3
On Fri, 30 Nov 2012 06:52:09 -0800
Joe Perches <joe@perches.com> wrote:

> On Fri, 2012-11-30 at 06:49 -0500, Jeff Layton wrote:
> > On Thu, 29 Nov 2012 11:37:19 -0800
> > Joe Perches <joe@perches.com> wrote:
> > 
> > > Make the compilation work again when CIFS_DEBUG is not #define'd.
> > > 
> > > Add format and argument verification for the various macros when
> > > CIFS_DEBUG is not #define'd.
> []
> > Would it be better to simply make those the standard
> > 
> > "do { ; } while(0)" noop macros?
> > 
> > I'm not sure I see the point in keeping the printk statements in there...
> 
> Actually, that's not standard.
> Look at printk.h
> 
> It prevents developers from adding code that compiles
> without warnings in one mode but compiles with warnings
> in another.
> 
> The idea is to make sure that format and arguments always
> match regardless of whether or not you are compiling
> debug or non-debug.
> 
> 

Fair enough then...

Reviewed-by: Jeff Layton <jlayton@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h
index b0fc344..4d12fe4 100644
--- a/fs/cifs/cifs_debug.h
+++ b/fs/cifs/cifs_debug.h
@@ -37,6 +37,9 @@  void dump_smb(void *, int);
 #define CIFS_RC		0x02
 #define CIFS_TIMER	0x04
 
+extern int cifsFYI;
+extern int cifsERROR;
+
 /*
  *	debug ON
  *	--------
@@ -44,36 +47,33 @@  void dump_smb(void *, int);
 #ifdef CIFS_DEBUG
 
 /* information message: e.g., configuration, major event */
-extern int cifsFYI;
-#define cifsfyi(fmt, arg...)						\
+#define cifsfyi(fmt, ...)						\
 do {									\
 	if (cifsFYI & CIFS_INFO)					\
-		printk(KERN_DEBUG "%s: " fmt "\n", __FILE__, ##arg);	\
+		printk(KERN_DEBUG "%s: " fmt "\n",			\
+		       __FILE__, ##__VA_ARGS__);			\
 } while (0)
 
-#define cFYI(set, fmt, arg...)			\
-do {						\
-	if (set)				\
-		cifsfyi(fmt, ##arg);		\
+#define cFYI(set, fmt, ...)						\
+do {									\
+	if (set)							\
+		cifsfyi(fmt, ##__VA_ARGS__);				\
 } while (0)
 
-#define cifswarn(fmt, arg...)			\
-	printk(KERN_WARNING fmt "\n", ##arg)
-
-/* debug event message: */
-extern int cifsERROR;
+#define cifswarn(fmt, ...)						\
+	printk(KERN_WARNING fmt "\n", ##__VA_ARGS__)
 
 /* error event message: e.g., i/o error */
-#define cifserror(fmt, arg...)					\
-do {								\
-	if (cifsERROR)						\
-		printk(KERN_ERR "CIFS VFS: " fmt "\n", ##arg);	\
+#define cifserror(fmt, ...)						\
+do {									\
+	if (cifsERROR)							\
+		printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__);	\
 } while (0)
 
-#define cERROR(set, fmt, arg...)		\
-do {						\
-	if (set)				\
-		cifserror(fmt, ##arg);		\
+#define cERROR(set, fmt, ...)						\
+do {									\
+	if (set)							\
+		cifserror(fmt, ##__VA_ARGS__);				\
 } while (0)
 
 /*
@@ -81,9 +81,27 @@  do {						\
  *	---------
  */
 #else		/* _CIFS_DEBUG */
-#define cERROR(set, fmt, arg...)
-#define cFYI(set, fmt, arg...)
-#define cifserror(fmt, arg...)
+#define cifsfyi(fmt, ...)						\
+do {									\
+	if (0)								\
+		printk(KERN_DEBUG "%s: " fmt "\n",			\
+		       __FILE__, ##__VA_ARGS__);			\
+} while (0)
+#define cFYI(set, fmt, ...)						\
+do {									\
+	if (0 && set)							\
+		cifsfyi(fmt, ##__VA_ARGS__);				\
+} while (0)
+#define cifserror(fmt, ...)						\
+do {									\
+	if (0)								\
+		printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__);	\
+} while (0)
+#define cERROR(set, fmt, ...)						\
+do {									\
+	if (0 && set)							\
+		cifserror(fmt, ##__VA_ARGS__);				\
+} while (0)
 #endif		/* _CIFS_DEBUG */
 
 #endif				/* _H_CIFS_DEBUG */