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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
