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

Reply via email to