public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: ft1000: fix some checkpatch complaints
@ 2014-07-27 17:08 Nicolas Thery
  2014-07-27 17:23 ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Thery @ 2014-07-27 17:08 UTC (permalink / raw)
  To: marek.belisko, gregkh, me; +Cc: devel, linux-kernel, nthery

This patch fixes the following errors and warnings:

ft1000_proc.c:26: WARNING: Use #include <linux/io.h> instead of <asm/io.h>
ft1000_proc.c:27: WARNING: Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
ft1000_proc.c:33: ERROR: Macros with multiple statements should be enclosed in a do - while loop
ft1000_proc.c:40: ERROR: Macros with multiple statements should be enclosed in a do - while loop
ft1000_proc.c:49: WARNING: static const char * array should probably be static const char * const
ft1000_proc.c:53: WARNING: static const char * array should probably be static const char * const

Signed-off-by: Nicolas Thery <nthery@gmail.com>
---
 drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
index 88f6f9c..28a2744 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
@@ -23,34 +23,38 @@
 #include <linux/string.h>
 #include <linux/vmalloc.h>
 #include <linux/netdevice.h>
-#include <asm/io.h>
-#include <asm/uaccess.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
 #include "ft1000.h"
 
 #define FT1000_PROC "ft1000"
 #define MAX_FILE_LEN 255
 
-#define seq_putx(m, message, size, var) \
+#define seq_putx(m, message, size, var) do { \
 	seq_printf(m, message);	\
 	for (i = 0; i < (size - 1); i++) { \
 		seq_printf(m, "%02x:", var[i]); \
 	} \
-	seq_printf(m, "%02x\n", var[i])
+	seq_printf(m, "%02x\n", var[i]); \
+} while (0)
 
-#define seq_putd(m, message, size, var) \
+#define seq_putd(m, message, size, var) do { \
 	seq_printf(m, message); \
 	for (i = 0; i < (size - 1); i++) { \
 		seq_printf(m, "%d.", var[i]); \
 	} \
-	seq_printf(m, "%d\n", var[i])
+	seq_printf(m, "%d\n", var[i]); \
+} while (0)
 
 static int ft1000ReadProc(struct seq_file *m, void *v)
 {
-	static const char *status[] = {
+	static const char * const status[] = {
 		"Idle (Disconnect)", "Searching", "Active (Connected)",
 		"Waiting for L2", "Sleep", "No Coverage", "", ""
 	};
-	static const char *signal[] = { "", "*", "**", "***", "****" };
+	static const char * const signal[] = {
+		"", "*", "**", "***", "****"
+	};
 
 	struct net_device *dev = m->private;
 	struct ft1000_info *info = netdev_priv(dev);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: ft1000: fix some checkpatch complaints
  2014-07-27 17:08 [PATCH] staging: ft1000: fix some checkpatch complaints Nicolas Thery
@ 2014-07-27 17:23 ` Joe Perches
  2014-07-27 17:35   ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2014-07-27 17:23 UTC (permalink / raw)
  To: Nicolas Thery; +Cc: marek.belisko, gregkh, me, devel, linux-kernel

On Sun, 2014-07-27 at 19:08 +0200, Nicolas Thery wrote:
> This patch fixes the following errors and warnings:
[]
> diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
[]
> @@ -23,34 +23,38 @@
[]
> -#define seq_putx(m, message, size, var) \
> +#define seq_putx(m, message, size, var) do { \
>  	seq_printf(m, message);	\
>  	for (i = 0; i < (size - 1); i++) { \
>  		seq_printf(m, "%02x:", var[i]); \
>  	} \
> -	seq_printf(m, "%02x\n", var[i])
> +	seq_printf(m, "%02x\n", var[i]); \
> +} while (0)

Ideally, these wouldn't depend on an external
i variable.

Maybe something like:

#define seq_putx(m, message, size, var) \
do { \
	int _i; \
 	seq_printf(m, message);	\
 	for (_i = 0; _i < (size - 1); _i++) \
 		seq_printf(m, "%02x:", var[_i]); \
	seq_printf(m, "%02x\n", var[_i]); \
} while (0)
 
> -#define seq_putd(m, message, size, var) \
> +#define seq_putd(m, message, size, var) do { \
>  	seq_printf(m, message); \
>  	for (i = 0; i < (size - 1); i++) { \
>  		seq_printf(m, "%d.", var[i]); \
>  	} \
> -	seq_printf(m, "%d\n", var[i])
> +	seq_printf(m, "%d\n", var[i]); \
> +} while (0)

Maybe later change these to use the recently introduced
seq_hex_dump: https://lkml.org/lkml/2014/7/11/269



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: ft1000: fix some checkpatch complaints
  2014-07-27 17:23 ` Joe Perches
@ 2014-07-27 17:35   ` Greg KH
  2014-07-27 17:40     ` Joe Perches
  2014-07-27 17:40     ` Nicolas Thery
  0 siblings, 2 replies; 5+ messages in thread
From: Greg KH @ 2014-07-27 17:35 UTC (permalink / raw)
  To: Joe Perches; +Cc: Nicolas Thery, marek.belisko, me, devel, linux-kernel

On Sun, Jul 27, 2014 at 10:23:53AM -0700, Joe Perches wrote:
> On Sun, 2014-07-27 at 19:08 +0200, Nicolas Thery wrote:
> > This patch fixes the following errors and warnings:
> []
> > diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
> []
> > @@ -23,34 +23,38 @@
> []
> > -#define seq_putx(m, message, size, var) \
> > +#define seq_putx(m, message, size, var) do { \
> >  	seq_printf(m, message);	\
> >  	for (i = 0; i < (size - 1); i++) { \
> >  		seq_printf(m, "%02x:", var[i]); \
> >  	} \
> > -	seq_printf(m, "%02x\n", var[i])
> > +	seq_printf(m, "%02x\n", var[i]); \
> > +} while (0)
> 
> Ideally, these wouldn't depend on an external
> i variable.
> 
> Maybe something like:
> 
> #define seq_putx(m, message, size, var) \
> do { \
> 	int _i; \
>  	seq_printf(m, message);	\
>  	for (_i = 0; _i < (size - 1); _i++) \
>  		seq_printf(m, "%02x:", var[_i]); \
> 	seq_printf(m, "%02x\n", var[_i]); \
> } while (0)
>  
> > -#define seq_putd(m, message, size, var) \
> > +#define seq_putd(m, message, size, var) do { \
> >  	seq_printf(m, message); \
> >  	for (i = 0; i < (size - 1); i++) { \
> >  		seq_printf(m, "%d.", var[i]); \
> >  	} \
> > -	seq_printf(m, "%d\n", var[i])
> > +	seq_printf(m, "%d\n", var[i]); \
> > +} while (0)
> 
> Maybe later change these to use the recently introduced
> seq_hex_dump: https://lkml.org/lkml/2014/7/11/269
> 

How about removing the proc files entirely, as I doubt they are really
needed :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: ft1000: fix some checkpatch complaints
  2014-07-27 17:35   ` Greg KH
@ 2014-07-27 17:40     ` Joe Perches
  2014-07-27 17:40     ` Nicolas Thery
  1 sibling, 0 replies; 5+ messages in thread
From: Joe Perches @ 2014-07-27 17:40 UTC (permalink / raw)
  To: Greg KH; +Cc: Nicolas Thery, marek.belisko, me, devel, linux-kernel

On Sun, 2014-07-27 at 10:35 -0700, Greg KH wrote:
> How about removing the proc files entirely, as I doubt they are really
> needed :)

unnecessary proc entry removals are always good too.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: ft1000: fix some checkpatch complaints
  2014-07-27 17:35   ` Greg KH
  2014-07-27 17:40     ` Joe Perches
@ 2014-07-27 17:40     ` Nicolas Thery
  1 sibling, 0 replies; 5+ messages in thread
From: Nicolas Thery @ 2014-07-27 17:40 UTC (permalink / raw)
  To: Greg KH; +Cc: Joe Perches, marek.belisko, devel, linux-kernel

On Sun, Jul 27, 2014 at 10:35:38AM -0700, Greg KH wrote:
> On Sun, Jul 27, 2014 at 10:23:53AM -0700, Joe Perches wrote:
> > On Sun, 2014-07-27 at 19:08 +0200, Nicolas Thery wrote:
> > > This patch fixes the following errors and warnings:
> > []
> > > diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
> > []
> > > @@ -23,34 +23,38 @@
> > []
> > > -#define seq_putx(m, message, size, var) \
> > > +#define seq_putx(m, message, size, var) do { \
> > >  	seq_printf(m, message);	\
> > >  	for (i = 0; i < (size - 1); i++) { \
> > >  		seq_printf(m, "%02x:", var[i]); \
> > >  	} \
> > > -	seq_printf(m, "%02x\n", var[i])
> > > +	seq_printf(m, "%02x\n", var[i]); \
> > > +} while (0)
> > 
> > Ideally, these wouldn't depend on an external
> > i variable.
> > 
> > Maybe something like:
> > 
> > #define seq_putx(m, message, size, var) \
> > do { \
> > 	int _i; \
> >  	seq_printf(m, message);	\
> >  	for (_i = 0; _i < (size - 1); _i++) \
> >  		seq_printf(m, "%02x:", var[_i]); \
> > 	seq_printf(m, "%02x\n", var[_i]); \
> > } while (0)
> >  
> > > -#define seq_putd(m, message, size, var) \
> > > +#define seq_putd(m, message, size, var) do { \
> > >  	seq_printf(m, message); \
> > >  	for (i = 0; i < (size - 1); i++) { \
> > >  		seq_printf(m, "%d.", var[i]); \
> > >  	} \
> > > -	seq_printf(m, "%d\n", var[i])
> > > +	seq_printf(m, "%d\n", var[i]); \
> > > +} while (0)
> > 
> > Maybe later change these to use the recently introduced
> > seq_hex_dump: https://lkml.org/lkml/2014/7/11/269
> > 
> 
> How about removing the proc files entirely, as I doubt they are really
> needed :)

Okay.  I'll do this.

Thanks to both of you for the review.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-07-27 17:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-27 17:08 [PATCH] staging: ft1000: fix some checkpatch complaints Nicolas Thery
2014-07-27 17:23 ` Joe Perches
2014-07-27 17:35   ` Greg KH
2014-07-27 17:40     ` Joe Perches
2014-07-27 17:40     ` Nicolas Thery

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox