From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753099AbbGBSBz (ORCPT ); Thu, 2 Jul 2015 14:01:55 -0400 Received: from smtprelay0081.hostedemail.com ([216.40.44.81]:47603 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753059AbbGBSBr (ORCPT ); Thu, 2 Jul 2015 14:01:47 -0400 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::,RULES_HIT:2:41:152:355:379:541:599:800:960:968:973:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1535:1593:1594:1605:1606:1730:1747:1777:1792:2393:2559:2562:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3874:4117:4250:4321:4605:5007:6119:6261:6691:7514:7903:9592:10004:10848:11026:11232:11473:11657:11658:11914:12043:12438:12517:12519:12555:12740:13894:21080:21212,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: shape33_f2b0c166b835 X-Filterd-Recvd-Size: 6621 Message-ID: <1435860103.2487.30.camel@perches.com> Subject: Re: [PATCH] defines modified to match the 80-char rule From: Joe Perches To: Mario Bambagini Cc: Krzysztof =?UTF-8?Q?Ha=C5=82asa?= , linux-kernel@vger.kernel.org, driverdev-devel@linuxdriverproject.org Date: Thu, 02 Jul 2015 11:01:43 -0700 In-Reply-To: <1435818026.2487.5.camel@perches.com> References: <1435094481-32275-1-git-send-email-mario.bambagini@gmail.com> <1435818026.2487.5.camel@perches.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11-0ubuntu3 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2015-07-01 at 23:20 -0700, Joe Perches wrote: > On Wed, 2015-07-01 at 09:59 +0200, Krzysztof Hałasa wrote: > > Mario Bambagini writes: > > > > > Defines have been written in more than one line to match the 80-character > > > rule. This error has been fixed 6 times in this file. > > > The file is fully compliant with respect to the coding rules now. > > > > Rules, maybe. But is it better, i.e., more readable? > > > > > --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h > > > +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h > [] > > > -#define LCONSOLE(mask, format, ...) CDEBUG(D_CONSOLE | (mask), format, ## __VA_ARGS__) > [] > > > +#define LCONSOLE(mask, format, ...) \ > > > + CDEBUG(D_CONSOLE | (mask), format, ## __VA_ARGS__) > > > ... I don't think so. Perhaps if I wasn't using the bleading edge tech > > 132-column digital flat LCD screen, I would see this differently (Emacs > > isn't perfect when displaying long lines on IBM monochrome display > > adapter, even with the intelligent-long-lines-wrap package). > > I think this isn't particularly nice because of the > different alignment styles used for the macros. > > I think it's OK as is, but it _might_ be nicer if it > removed the space after ## and used the same indent > as most other macros. I suggest something like this: o Use fmt for format o Consistent use of ##__VA_ARGS__ o Consistent argument alignment o Consistent line continuation alignment o Standardize __printf location Perhaps the libcfs_debug_msg and libcfs_debug_vmsg2 functions can be collapsed into a single function using the vsprintf %pV extension. --- .../lustre/include/linux/libcfs/libcfs_debug.h | 54 +++++++++++++--------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h index 8251ac9..97a8710 100644 --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h @@ -207,7 +207,7 @@ static inline int cfs_cdebug_show(unsigned int mask, unsigned int subsystem) ((libcfs_debug & mask) && (libcfs_subsystem_debug & subsystem)); } -#define __CDEBUG(cdls, mask, format, ...) \ +#define __CDEBUG(cdls, mask, fmt, ...) \ do { \ static struct libcfs_debug_msg_data msgdata; \ \ @@ -215,41 +215,51 @@ do { \ \ if (cfs_cdebug_show(mask, DEBUG_SUBSYSTEM)) { \ LIBCFS_DEBUG_MSG_DATA_INIT(&msgdata, mask, cdls); \ - libcfs_debug_msg(&msgdata, format, ## __VA_ARGS__); \ + libcfs_debug_msg(&msgdata, fmt, ##__VA_ARGS__); \ } \ } while (0) -#define CDEBUG(mask, format, ...) __CDEBUG(NULL, mask, format, ## __VA_ARGS__) +#define CDEBUG(mask, fmt, ...) \ + __CDEBUG(NULL, mask, fmt, ##__VA_ARGS__) -#define CDEBUG_LIMIT(mask, format, ...) \ +#define CDEBUG_LIMIT(mask, fmt, ...) \ do { \ static struct cfs_debug_limit_state cdls; \ \ - __CDEBUG(&cdls, mask, format, ## __VA_ARGS__); \ + __CDEBUG(&cdls, mask, fmt, ##__VA_ARGS__); \ } while (0) -#define CWARN(format, ...) CDEBUG_LIMIT(D_WARNING, format, ## __VA_ARGS__) -#define CERROR(format, ...) CDEBUG_LIMIT(D_ERROR, format, ## __VA_ARGS__) -#define CNETERR(format, a...) CDEBUG_LIMIT(D_NETERROR, format, ## a) -#define CEMERG(format, ...) CDEBUG_LIMIT(D_EMERG, format, ## __VA_ARGS__) +#define CWARN(fmt, ...) \ + CDEBUG_LIMIT(D_WARNING, fmt, ##__VA_ARGS__) +#define CERROR(fmt, ...) \ + CDEBUG_LIMIT(D_ERROR, fmt, ##__VA_ARGS__) +#define CNETERR(fmt, ...) \ + CDEBUG_LIMIT(D_NETERROR, fmt, ##__VA_ARGS__) +#define CEMERG(fmt, ...) \ + CDEBUG_LIMIT(D_EMERG, fmt, ##__VA_ARGS__) -#define LCONSOLE(mask, format, ...) CDEBUG(D_CONSOLE | (mask), format, ## __VA_ARGS__) -#define LCONSOLE_INFO(format, ...) CDEBUG_LIMIT(D_CONSOLE, format, ## __VA_ARGS__) -#define LCONSOLE_WARN(format, ...) CDEBUG_LIMIT(D_CONSOLE | D_WARNING, format, ## __VA_ARGS__) -#define LCONSOLE_ERROR_MSG(errnum, format, ...) CDEBUG_LIMIT(D_CONSOLE | D_ERROR, \ - "%x-%x: " format, errnum, LERRCHKSUM(errnum), ## __VA_ARGS__) -#define LCONSOLE_ERROR(format, ...) LCONSOLE_ERROR_MSG(0x00, format, ## __VA_ARGS__) +#define LCONSOLE(mask, fmt, ...) \ + CDEBUG(D_CONSOLE | (mask), fmt, ##__VA_ARGS__) +#define LCONSOLE_INFO(fmt, ...) \ + CDEBUG_LIMIT(D_CONSOLE, fmt, ##__VA_ARGS__) +#define LCONSOLE_WARN(fmt, ...) \ + CDEBUG_LIMIT(D_CONSOLE | D_WARNING, fmt, ##__VA_ARGS__) +#define LCONSOLE_ERROR_MSG(errnum, fmt, ...) \ + CDEBUG_LIMIT(D_CONSOLE | D_ERROR, "%x-%x: " fmt, \ + errnum, LERRCHKSUM(errnum), ##__VA_ARGS__) +#define LCONSOLE_ERROR(fmt, ...) \ + LCONSOLE_ERROR_MSG(0x00, fmt, ##__VA_ARGS__) -#define LCONSOLE_EMERG(format, ...) CDEBUG(D_CONSOLE | D_EMERG, format, ## __VA_ARGS__) +#define LCONSOLE_EMERG(fmt, ...) \ + CDEBUG(D_CONSOLE | D_EMERG, fmt, ##__VA_ARGS__) +__printf(2, 3) int libcfs_debug_msg(struct libcfs_debug_msg_data *msgdata, - const char *format1, ...) - __printf(2, 3); - + const char *fmt, ...); +__printf(4, 5) int libcfs_debug_vmsg2(struct libcfs_debug_msg_data *msgdata, - const char *format1, - va_list args, const char *format2, ...) - __printf(4, 5); + const char *format1, va_list args, + const char *format2, ...); /* other external symbols that tracefile provides: */ int cfs_trace_copyin_string(char *knl_buffer, int knl_buffer_nob,