From: Joe Perches <joe@perches.com> To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>, 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 Subject: Re: [PATCH 2/2 v3] be2iscsi: Fix some error messages Date: Sat, 13 Aug 2016 10:03:02 -0700 [thread overview] Message-ID: <1471107782.3467.28.camel@perches.com> (raw) In-Reply-To: <1471106498.3467.19.camel@perches.com> 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.
WARNING: multiple messages have this Message-ID (diff)
From: Joe Perches <joe@perches.com> To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>, 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 Subject: Re: [PATCH 2/2 v3] be2iscsi: Fix some error messages Date: Sat, 13 Aug 2016 17:03:02 +0000 [thread overview] Message-ID: <1471107782.3467.28.camel@perches.com> (raw) In-Reply-To: <1471106498.3467.19.camel@perches.com> 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. -- 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
next prev parent reply other threads:[~2016-08-13 17:03 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-08-12 10:02 [PATCH 2/2] be2iscsi: Fix some error messages Christophe JAILLET 2016-08-12 10:02 ` Christophe JAILLET 2016-08-12 10:30 ` Julia Lawall 2016-08-12 10:30 ` Julia Lawall 2016-08-12 20:30 ` Christophe JAILLET 2016-08-12 20:30 ` Christophe JAILLET 2016-08-13 7:08 ` [PATCH 2/2 v2] " Christophe JAILLET 2016-08-13 7:08 ` Christophe JAILLET 2016-08-13 7:14 ` Julia Lawall 2016-08-13 7:14 ` Julia Lawall 2016-08-13 7:20 ` [PATCH 2/2 v3] " Christophe JAILLET 2016-08-13 7:20 ` Christophe JAILLET 2016-08-13 11:35 ` Joe Perches 2016-08-13 11:35 ` Joe Perches 2016-08-13 12:31 ` Christophe JAILLET 2016-08-13 12:31 ` Christophe JAILLET 2016-08-13 16:41 ` Joe Perches 2016-08-13 16:41 ` Joe Perches 2016-08-13 17:03 ` Joe Perches [this message] 2016-08-13 17:03 ` Joe Perches 2016-08-13 20:42 ` [PATCH 0/2] be2iscsi: Logging neatening Joe Perches 2016-08-13 20:42 ` [PATCH 1/2] be2iscsi: Coalesce split strings and formats Joe Perches 2016-08-13 20:42 ` [PATCH 2/2] be2iscsi: Use a standard logging style Joe Perches 2016-08-14 14:34 ` [PATCH 0/2] be2iscsi: Logging neatening Bart Van Assche 2016-08-14 16:24 ` Joe Perches 2016-08-14 17:09 ` Bart Van Assche 2016-08-14 17:29 ` Joe Perches 2016-08-17 1:19 ` Bart Van Assche 2016-08-17 3:39 ` Joe Perches 2016-08-14 18:55 ` [PATCH] be2iscsi: Use a more current logging style Joe Perches 2016-08-16 6:02 ` Jitendra Bhivare 2016-08-16 10:27 ` Joe Perches 2016-08-17 3:50 ` Jitendra Bhivare 2016-08-17 3:59 ` Joe Perches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1471107782.3467.28.camel@perches.com \ --to=joe@perches.com \ --cc=christophe.jaillet@wanadoo.fr \ --cc=jayamohan.kallickal@avagotech.com \ --cc=jejb@linux.vnet.ibm.com \ --cc=kernel-janitors@vger.kernel.org \ --cc=ketan.mukadam@avagotech.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=sony.john@avagotech.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.