From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752805AbcHMRDJ (ORCPT ); Sat, 13 Aug 2016 13:03:09 -0400 Received: from smtprelay0022.hostedemail.com ([216.40.44.22]:33395 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752694AbcHMRDH (ORCPT ); Sat, 13 Aug 2016 13:03:07 -0400 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 30,2,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::::::::,RULES_HIT:41:355:379:541:599:960:973:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1534:1542:1593:1594:1711:1730:1747:1777:1792:2393:2559:2562:2828:3138:3139:3140:3141:3142:3353:3622:3865:3867:3868:3870:3871:3872:3874:4250:4321:5007:6691:10010:10400:10848:11026:11232:11473:11658:11783:11914:12043:12048:12296:12438:12517:12519:12555:12679:12740:13439:13894:14659:14721:21080:21212:21433:21451:30054:30064:30091,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:1:0,LFtime:5,LUA_SUMMARY:none X-HE-Tag: quilt66_7207fc627752c X-Filterd-Recvd-Size: 3192 Message-ID: <1471107782.3467.28.camel@perches.com> Subject: Re: [PATCH 2/2 v3] be2iscsi: Fix some error messages From: Joe Perches To: Christophe JAILLET , jayamohan.kallickal@avagotech.com, jejb@linux.vnet.ibm.com, ketan.mukadam@avagotech.com, sony.john@avagotech.com, martin.petersen@oracle.com Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Date: Sat, 13 Aug 2016 10:03:02 -0700 In-Reply-To: <1471106498.3467.19.camel@perches.com> References: <4cd24bc3-7953-479a-1dbe-2aac9299ce13@wanadoo.fr> <1471072824-1491-1-git-send-email-christophe.jaillet@wanadoo.fr> <1471088144.3467.11.camel@perches.com> <1471106498.3467.19.camel@perches.com> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.18.5.2-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 Sat, 2016-08-13 at 09:41 -0700, Joe Perches wrote: > On Sat, 2016-08-13 at 14:31 +0200, Christophe JAILLET wrote: > > Le 13/08/2016 à 13:35, Joe Perches a écrit : > > > > @@ -268,7 +268,7 @@ static int beiscsi_eh_abort(struct scsi_cmnd *sc) > > > >    &nonemb_cmd.dma); > > > >    if (nonemb_cmd.va == NULL) { > > > >    beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_EH, > > > > -     "BM_%d : Failed to allocate memory for" > > > > +     "BM_%d : Failed to allocate memory for " > > > >        "mgmt_invalidate_icds\n"); This is the first time I've looked at the beiscsi_log macro. It sure is odd and undesirable. It's _very_ not nice to have a format string take an implied __LINE__ argument. It'd be much more intelligible to take the first bit as a separate string, concatenate it in the macro with "_%d: " and __LINE__ (if that's really useful, I think it's not) and emit that as the format. Something like: diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h index 30a4606..3f0fbbf 100644 --- a/drivers/scsi/be2iscsi/be_main.h +++ b/drivers/scsi/be2iscsi/be_main.h @@ -1084,11 +1084,12 @@ struct hwi_context_memory {  #define __beiscsi_log(phba, level, fmt, arg...) \   shost_printk(level, phba->shost, fmt, __LINE__, ##arg)   -#define beiscsi_log(phba, level, mask, fmt, arg...) \ -do { \ - uint32_t log_value = phba->attr_log_enable; \ - if (((mask) & log_value) || (level[1] <= '3')) \ - __beiscsi_log(phba, level, fmt, ##arg); \ -} while (0); +#define beiscsi_log(phba, level, mask, prefix, fmt, ...) \ +do { \ + uint32_t log_value = phba->attr_log_enable; \ + if (((mask) & log_value) || (level[1] <= '3')) \ + __beiscsi_log(phba, level, prefix "_%d: " fmt, \ +       ##__VA_ARGS__); \ +} while (0)    #endif So these beiscsi_log uses become something like: beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_EH,     "BM", "Failed to allocate memory for mgmt_invalidate_icds\n"); and the format and its arguments match. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Perches Date: Sat, 13 Aug 2016 17:03:02 +0000 Subject: Re: [PATCH 2/2 v3] be2iscsi: Fix some error messages Message-Id: <1471107782.3467.28.camel@perches.com> List-Id: References: <4cd24bc3-7953-479a-1dbe-2aac9299ce13@wanadoo.fr> <1471072824-1491-1-git-send-email-christophe.jaillet@wanadoo.fr> <1471088144.3467.11.camel@perches.com> <1471106498.3467.19.camel@perches.com> In-Reply-To: <1471106498.3467.19.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Christophe JAILLET , jayamohan.kallickal@avagotech.com, jejb@linux.vnet.ibm.com, ketan.mukadam@avagotech.com, sony.john@avagotech.com, martin.petersen@oracle.com Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org On Sat, 2016-08-13 at 09:41 -0700, Joe Perches wrote: > On Sat, 2016-08-13 at 14:31 +0200, Christophe JAILLET wrote: > > Le 13/08/2016 =E0 13:35, Joe Perches a =E9crit : > > > > @@ -268,7 +268,7 @@ static int beiscsi_eh_abort(struct scsi_cmnd *s= c) > > > > =A0=A0 &nonemb_cmd.dma); > > > > =A0=A0 if (nonemb_cmd.va =3D NULL) { > > > > =A0=A0 beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_EH, > > > > - =A0=A0=A0=A0"BM_%d : Failed to allocate memory for" > > > > + =A0=A0=A0=A0"BM_%d : Failed to allocate memory for " > > > > =A0=A0 =A0=A0=A0=A0"mgmt_invalidate_icds\n"); This is the first time I've looked at the beiscsi_log macro. It sure is odd and undesirable. It's _very_ not nice to have a format string take an implied __LINE__ argument. It'd be much more intelligible to take the first bit as a separate string, concatenate it in the macro with "_%d: " and __LINE__ (if that's really useful, I think it's not) and emit that as the format. Something like: diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_mai= n.h index 30a4606..3f0fbbf 100644 --- a/drivers/scsi/be2iscsi/be_main.h +++ b/drivers/scsi/be2iscsi/be_main.h @@ -1084,11 +1084,12 @@ struct hwi_context_memory { =A0#define __beiscsi_log(phba, level, fmt, arg...) \ =A0 shost_printk(level, phba->shost, fmt, __LINE__, ##arg) =A0 -#define beiscsi_log(phba, level, mask, fmt, arg...) \ -do { \ - uint32_t log_value =3D phba->attr_log_enable; \ - if (((mask) & log_value) || (level[1] <=3D '3')) \ - __beiscsi_log(phba, level, fmt, ##arg); \ -} while (0); +#define beiscsi_log(phba, level, mask, prefix, fmt, ...) \ +do { \ + uint32_t log_value =3D phba->attr_log_enable; \ + if (((mask) & log_value) || (level[1] <=3D '3')) \ + __beiscsi_log(phba, level, prefix "_%d: " fmt, \ + =A0=A0=A0=A0=A0=A0##__VA_ARGS__); \ +} while (0) =A0 =A0#endif So these beiscsi_log uses become something like: beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_EH, =A0=A0=A0=A0"BM", "Failed to allocate memory for mgmt_invalidate_icds\n"); and the format and its arguments match. -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" = in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html