staging:comedi: Fixed coding convention issues.
in [Kernel]
|
From: Mark Rankilor on 12 Jun 2010 22:20 2010/6/12 Henri H�kkinen <henuxd(a)gmail.com>: > - � � � � � � � � � � � printk > - � � � � � � � � � � � � � (KERN_INFO "comedi: failed to increment module count, skipping\n"); > + � � � � � � � � � � � printk(KERN_INFO "comedi: failed to increment module " > + � � � � � � � � � � � � � � �"count, skipping\n"); Hi Henri, Regarding your breaking up of printk statements, although some of those lines do go over 80 characters, it is preferable to keep the strings together since then those are searchable within the code. I figure it is quite acceptable to break the string after "comedi: ", so maybe that will fix the line length issue, otherwise it is preferable to keep the checkpatch warning in this case. Mark -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Joe Perches on 13 Jun 2010 01:10 On Sun, 2010-06-13 at 10:14 +0800, Mark Rankilor wrote: > 2010/6/12 Henri Häkkinen <henuxd(a)gmail.com>: > > - printk > > - (KERN_INFO "comedi: failed to increment module count, skipping\n"); > > + printk(KERN_INFO "comedi: failed to increment module " > > + "count, skipping\n"); > > Regarding your breaking up of printk statements, although some of > those lines do go over 80 characters, it is preferable to keep the > strings together since then those are searchable within the code. > > I figure it is quite acceptable to break the string after "comedi: ", > so maybe that will fix the line length issue, otherwise it is > preferable to keep the checkpatch warning in this case. A couple of options for comedi: 1: Use #define pr_fmt(fmt) "comedi: " fmt pr_<level>(format, ...) 2: Create some comedi logging functions or macros like: comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc) where "comedi:" is always prefixed and an optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt could be used. That'd shorten line lengths quite a bit and add some better standardization to comedi. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Joe Perches on 13 Jun 2010 01:40 On Sat, 2010-06-12 at 22:07 -0700, Joe Perches wrote: > 2: Create some comedi logging functions or macros like: > comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc) > where "comedi:" is always prefixed and an > optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > could be used. Maybe this is a start: Signed-off-by: Joe Perches <joe(a)perches.com> --- drivers/staging/comedi/comedidev.h | 54 ++++++++++++++++++++++++++++++++++-- 1 files changed, 51 insertions(+), 3 deletions(-) diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h index 4eb2b77..6c2bdde 100644 --- a/drivers/staging/comedi/comedidev.h +++ b/drivers/staging/comedi/comedidev.h @@ -43,11 +43,59 @@ #include "comedi.h" -#define DPRINTK(format, args...) do { \ - if (comedi_debug) \ - printk(KERN_DEBUG "comedi: " format , ## args); \ +#define comedi_printk(level, fmt, args...) \ + printk(level "comedi: " pr_fmt(fmt), ##args) + +#define DPRINTK(format, args...) \ +do { \ + if (comedi_debug) \ + comedi_printk(KERN_DEBUG, fmt, ##args); \ } while (0) +#define comedi_emerg(fmt, ...) \ + comedi_printk(KERN_EMERG, fmt, ##__VA_ARGS__) +#define comedi_alert(fmt, ...) \ + comedi_printk(KERN_ALERT, fmt, ##__VA_ARGS__) +#define comedi_crit(fmt, ...) \ + comedi_printk(KERN_CRIT, fmt, ##__VA_ARGS__) +#define comedi_err(fmt, ...) \ + comedi_printk(KERN_ERR, fmt, ##__VA_ARGS__) +#define comedi_warn(fmt, ...) \ + comedi_printk(KERN_WARNING, fmt, ##__VA_ARGS__) +#define comedi_notice(fmt, ...) \ + comedi_printk(KERN_NOTICE, fmt, ##__VA_ARGS__) +#define comedi_info(fmt, ...) \ + comedi_printk(KERN_INFO, fmt, ##__VA_ARGS__) + +/* comedi_devel() should produce zero code unless DEBUG is defined */ +#ifdef DEBUG +#define comedi_devel(fmt, ...) \ + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__) +#else +#define comedi_devel(fmt, ...) \ +({ \ + if (0) \ + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__); \ + 0; \ +}) +#endif + +#if defined(DEBUG) +#define comedi_debug(fmt, ...) \ + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__) +#elif defined(CONFIG_DYNAMIC_DEBUG) +/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */ +#define comedi_debug(fmt, ...) \ + dynamic_pr_debug(fmt, ##__VA_ARGS__) +#else +#define comedi_debug(fmt, ...) \ +({ \ + if (0) \ + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__); \ + 0; \ +}) +#endif + #define COMEDI_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c)) #define COMEDI_VERSION_CODE COMEDI_VERSION(COMEDI_MAJORVERSION, \ COMEDI_MINORVERSION, COMEDI_MICROVERSION) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Henri Häkkinen on 13 Jun 2010 07:30 Hello There are several printk statements without the "comedi:" prefix. Such as: printk(KERN_WARNING "BUG: dev->driver=NULL in comedi_device_detach()\n"); Do you think it is better to leave these as they are, or should they be changed to use comedi_xxx macros (which will print the "comedi:" prefix)? Also even with logging macros, there will be few lines which go beyond the 80 character boundary. On 13.6.2010, at 8.30, Joe Perches wrote: > On Sat, 2010-06-12 at 22:07 -0700, Joe Perches wrote: >> 2: Create some comedi logging functions or macros like: >> comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc) >> where "comedi:" is always prefixed and an >> optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> could be used. > > Maybe this is a start: > > Signed-off-by: Joe Perches <joe(a)perches.com> > --- > drivers/staging/comedi/comedidev.h | 54 ++++++++++++++++++++++++++++++++++-- > 1 files changed, 51 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h > index 4eb2b77..6c2bdde 100644 > --- a/drivers/staging/comedi/comedidev.h > +++ b/drivers/staging/comedi/comedidev.h > @@ -43,11 +43,59 @@ > > #include "comedi.h" > > -#define DPRINTK(format, args...) do { \ > - if (comedi_debug) \ > - printk(KERN_DEBUG "comedi: " format , ## args); \ > +#define comedi_printk(level, fmt, args...) \ > + printk(level "comedi: " pr_fmt(fmt), ##args) > + > +#define DPRINTK(format, args...) \ > +do { \ > + if (comedi_debug) \ > + comedi_printk(KERN_DEBUG, fmt, ##args); \ > } while (0) > > +#define comedi_emerg(fmt, ...) \ > + comedi_printk(KERN_EMERG, fmt, ##__VA_ARGS__) > +#define comedi_alert(fmt, ...) \ > + comedi_printk(KERN_ALERT, fmt, ##__VA_ARGS__) > +#define comedi_crit(fmt, ...) \ > + comedi_printk(KERN_CRIT, fmt, ##__VA_ARGS__) > +#define comedi_err(fmt, ...) \ > + comedi_printk(KERN_ERR, fmt, ##__VA_ARGS__) > +#define comedi_warn(fmt, ...) \ > + comedi_printk(KERN_WARNING, fmt, ##__VA_ARGS__) > +#define comedi_notice(fmt, ...) \ > + comedi_printk(KERN_NOTICE, fmt, ##__VA_ARGS__) > +#define comedi_info(fmt, ...) \ > + comedi_printk(KERN_INFO, fmt, ##__VA_ARGS__) > + > +/* comedi_devel() should produce zero code unless DEBUG is defined */ > +#ifdef DEBUG > +#define comedi_devel(fmt, ...) \ > + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__) > +#else > +#define comedi_devel(fmt, ...) \ > +({ \ > + if (0) \ > + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__); \ > + 0; \ > +}) > +#endif > + > +#if defined(DEBUG) > +#define comedi_debug(fmt, ...) \ > + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__) > +#elif defined(CONFIG_DYNAMIC_DEBUG) > +/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */ > +#define comedi_debug(fmt, ...) \ > + dynamic_pr_debug(fmt, ##__VA_ARGS__) > +#else > +#define comedi_debug(fmt, ...) \ > +({ \ > + if (0) \ > + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__); \ > + 0; \ > +}) > +#endif > + > #define COMEDI_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c)) > #define COMEDI_VERSION_CODE COMEDI_VERSION(COMEDI_MAJORVERSION, \ > COMEDI_MINORVERSION, COMEDI_MICROVERSION) > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Joe Perches on 13 Jun 2010 14:20 On Sun, 2010-06-13 at 14:27 +0300, Henri Häkkinen wrote: > There are several printk statements without the "comedi:" prefix. > Do you think it is better to leave these as they are, or should they > be changed to use comedi_xxx macros (which will print the "comedi:" > prefix)? > > printk(KERN_WARNING "BUG: dev->driver=NULL in comedi_device_detach()\n"); I think it's better to convert them. Anything with "BUG" in the format maybe should be converted to BUG_ON. > Also even with logging macros, there will be few lines which go beyond > the 80 character boundary. I'd ignore printk related long line warnings. I suggest coalescing the format string to a single line where reasonable. If a single printk has non trailing '\n's in a format, it may be better to split them up. comedi_info("some incredibly long output line with error: %d\n" "Another line with some other information: %d\n", err, info); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Next
|
Last
Pages: 1 2 3 Prev: checkpatch.pl skip long lines Next: [PATCH] vmlinux.lds.h: lower init ramfs alignment to 4 |