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 linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html