On 10/03/2014 06:31 PM, Ewan Milne wrote: > On Thu, 2014-10-02 at 18:37 +0000, Elliott, Robert (Server Storage) > wrote: >>> -----Original Message----- >>> From: Hannes Reinecke [mailto:hare@xxxxxxx] >> ... >>> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h >>> index 4c3ab83..c01dc89 100644 >>> --- a/drivers/scsi/sd.h >>> +++ b/drivers/scsi/sd.h >>> @@ -103,9 +103,10 @@ static inline struct scsi_disk *scsi_disk(struct gendisk >>> *disk) >>> >>> #define sd_printk(prefix, sdsk, fmt, a...) \ >>> (sdsk)->disk ? \ >>> - sdev_printk(prefix, (sdsk)->device, "[%s] " fmt, \ >>> - (sdsk)->disk->disk_name, ##a) : \ >>> - sdev_printk(prefix, (sdsk)->device, fmt, ##a) >>> + sdev_prefix_printk(prefix, (sdsk)->device, \ >>> + (sdsk)->disk->disk_name, fmt, ##a) : \ >>> + sdev_prefix_printk(prefix, (sdsk)->device, \ >>> + NULL, fmt, ##a) >>> >>> #define sd_first_printk(prefix, sdsk, fmt, a...) \ >>> do { \ >> ... >>> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h >>> index 27ecee7..0b18a09 100644 >>> --- a/include/scsi/scsi_device.h >>> +++ b/include/scsi/scsi_device.h >>> @@ -244,6 +244,15 @@ struct scsi_dh_data { >>> #define sdev_dbg(sdev, fmt, a...) \ >>> dev_dbg(&(sdev)->sdev_gendev, fmt, ##a) >>> >>> +/* >>> + * like scmd_printk, but the device name is passed in >>> + * as a string pointer >>> + */ >>> +#define sdev_prefix_printk(l, sdev, p, fmt, a...) \ >>> + (p) ? \ >>> + sdev_printk(l, sdev, "[%s] " fmt, p, ##a) : \ >>> + sdev_printk(l, sdev, fmt, ##a) >>> + >>> #define scmd_printk(prefix, scmd, fmt, a...) \ >>> (scmd)->request->rq_disk ? \ >>> sdev_printk(prefix, (scmd)->device, "[%s] " fmt, \ >>> -- >>> 1.8.5.2 >> >> This triggers lots of compiler warnings with gcc 4.4.7 like: >> >> drivers/scsi/sd.c: In function 'sd_open': >> drivers/scsi/sd.c:1179: warning: reading through null pointer (argument 4) >> drivers/scsi/sd.c:1179: warning: format '%s' expects type 'char *', but argument 4 has type 'void *' >> >> >> That is from: >> SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n")); >> >> Since: >> #define NULL ((void *)0) >> >> gcc probably doesn't realize the (p)? prevents the NULL (a void *) >> from being passed to sdev_printk. >> >> Passing "" rather than NULL eliminates the compiler warnings. > > It eliminates the warnings, but unfortunately we then get log messages > that look like: > > Oct 3 11:30:08 rhel-storage-01 kernel: sd 10:0:0:0: [sde] (null)FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > > ^^^^^^ > > Changing it to (char *)NULL, like this: > > #define sd_printk(prefix, sdsk, fmt, a...) \ > (sdsk)->disk ? \ > sdev_prefix_printk(prefix, (sdsk)->device, \ > (sdsk)->disk->disk_name, fmt, ##a) : \ > sdev_prefix_printk(prefix, (sdsk)->device, \ > (char *)NULL, fmt, ##a) > > doesn't work either. The compiler gives an error: > > drivers/scsi/sd.c: In function 'sd_open': > drivers/scsi/sd.c:1158:2: error: reading through null pointer (argument 4) [-Werror=format=] > SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n")); > ^ > I've fixed it up in my next iteration of the patchset. (By simply using 'sdev_printk' if the prefix argument is NULL ...) Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@xxxxxxx +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
