[PATCH 1/4] scsi: always use format argumets for dev_printk

[PATCH 1/4] scsi: always use format argumets for dev_printk

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_logging.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c
index 6128303..ecc5918 100644
--- a/drivers/scsi/scsi_logging.c
+++ b/drivers/scsi/scsi_logging.c
@@ -251,7 +251,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
 	if (cmd->cmd_len > 16) {
 		/* Print opcode in one line and use separate lines for CDB */
 		off += scnprintf(logbuf + off, logbuf_len - off, "\n");
-		dev_printk(KERN_INFO, &cmd->device->sdev_gendev, logbuf);
+		dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf);
 		scsi_log_release_buffer(logbuf);
 		for (k = 0; k < cmd->cmd_len; k += 16) {
 			size_t linelen = min(cmd->cmd_len - k, 16);
@@ -269,7 +269,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
 						   16, 1, logbuf + off,
 						   logbuf_len - off, false);
 			}
-			dev_printk(KERN_INFO, &cmd->device->sdev_gendev,
+			dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s",
 				   logbuf);
 			scsi_log_release_buffer(logbuf);
 		}
@@ -282,7 +282,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
 				   false);
 	}
 out_printk:
-	dev_printk(KERN_INFO, &cmd->device->sdev_gendev, logbuf);
+	dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf);
 	scsi_log_release_buffer(logbuf);
 }
 EXPORT_SYMBOL(scsi_print_command);
@@ -360,7 +360,7 @@ scsi_log_dump_sense(const struct scsi_device *sdev, const char *name, int tag,
 		hex_dump_to_buffer(&sense_buffer[i], len, 16, 1,
 				   logbuf + off, logbuf_len - off,
 				   false);
-		dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
+		dev_printk(KERN_INFO, &sdev->sdev_gendev, "%s", logbuf);
 	}
 	scsi_log_release_buffer(logbuf);
 }
@@ -377,7 +377,7 @@ scsi_log_print_sense_hdr(const struct scsi_device *sdev, const char *name,
 		return;
 	off = sdev_format_header(logbuf, logbuf_len, name, tag);
 	off += scsi_format_sense_hdr(logbuf + off, logbuf_len - off, sshdr);
-	dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
+	dev_printk(KERN_INFO, &sdev->sdev_gendev, "%s", logbuf);
 	scsi_log_release_buffer(logbuf);
 
 	logbuf = scsi_log_reserve_buffer(&logbuf_len);
@@ -386,7 +386,7 @@ scsi_log_print_sense_hdr(const struct scsi_device *sdev, const char *name,
 	off = sdev_format_header(logbuf, logbuf_len, name, tag);
 	off += scsi_format_extd_sense(logbuf + off, logbuf_len - off,
 				      sshdr->asc, sshdr->ascq);
-	dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
+	dev_printk(KERN_INFO, &sdev->sdev_gendev, "%s", logbuf);
 	scsi_log_release_buffer(logbuf);
 }
 
@@ -483,7 +483,7 @@ void scsi_print_result(const struct scsi_cmnd *cmd, const char *msg,
 		off += scnprintf(logbuf + off, logbuf_len - off,
 				 "driverbyte=0x%02x", driver_byte(cmd->result));
 out_printk:
-	dev_printk(KERN_INFO, &cmd->device->sdev_gendev, logbuf);
+	dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf);
 	scsi_log_release_buffer(logbuf);
 }
 EXPORT_SYMBOL(scsi_print_result);
-- 
1.9.1

[PATCH 2/4] scsi: annotate sdev_prefix_printk and scmd_printk as printf-like

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/scsi/scsi_device.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d1aad4d..2e0281e 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -237,14 +237,15 @@ struct scsi_dh_data {
  * like scmd_printk, but the device name is passed in
  * as a string pointer
  */
-extern int sdev_prefix_printk(const char *, const struct scsi_device *,
-			      const char *, const char *, ...);
+__printf(4, 5) int
+sdev_prefix_printk(const char *, const struct scsi_device *, const char *,
+		const char *, ...);
 
 #define sdev_printk(l, sdev, fmt, a...)				\
 	sdev_prefix_printk(l, sdev, NULL, fmt, ##a)
 
-extern int scmd_printk(const char *, const struct scsi_cmnd *,
-		       const char *, ...);
+__printf(3, 4) int
+scmd_printk(const char *, const struct scsi_cmnd *, const char *, ...);
 
 #define scmd_dbg(scmd, fmt, a...)					   \
 	do {								   \
-- 
1.9.1

Re: [PATCH 2/4] scsi: annotate sdev_prefix_printk and scmd_printk as printf-like

[Hannes Reinecke]

On 01/18/2015 04:11 PM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] scsi: annotate sdev_prefix_printk and scmd_printk as printf-like

[Ewan Milne]

On Sun, 2015-01-18 at 16:11 +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/scsi/scsi_device.h | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index d1aad4d..2e0281e 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -237,14 +237,15 @@ struct scsi_dh_data {
>   * like scmd_printk, but the device name is passed in
>   * as a string pointer
>   */
> -extern int sdev_prefix_printk(const char *, const struct scsi_device *,
> -			      const char *, const char *, ...);
> +__printf(4, 5) int
> +sdev_prefix_printk(const char *, const struct scsi_device *, const char *,
> +		const char *, ...);
>  
>  #define sdev_printk(l, sdev, fmt, a...)				\
>  	sdev_prefix_printk(l, sdev, NULL, fmt, ##a)
>  
> -extern int scmd_printk(const char *, const struct scsi_cmnd *,
> -		       const char *, ...);
> +__printf(3, 4) int
> +scmd_printk(const char *, const struct scsi_cmnd *, const char *, ...);
>  
>  #define scmd_dbg(scmd, fmt, a...)					   \
>  	do {								   \

Reviewed-by: Ewan D. Milne <emilne@redhat.com>

[PATCH 3/4] ch: remove debug noise in ch_do_scsi

[Christoph Hellwig]

The midlayer logging already prints the cdb details if the logging
level is high enough, no need to duplicate this in the ch driver.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/ch.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index 79e462f..0045742 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -194,18 +194,10 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd, int cmd_len,
 
  retry:
 	errno = 0;
-	if (debug) {
-		char logbuf[SCSI_LOG_BUFSIZE];
-
-		__scsi_format_command(logbuf, sizeof(logbuf), cmd, cmd_len);
-		DPRINTK("command: %s", logbuf);
-	}
-
 	result = scsi_execute_req(ch->device, cmd, direction, buffer,
 				  buflength, &sshdr, timeout * HZ,
 				  MAX_RETRIES, NULL);
 
-	DPRINTK("result: 0x%x\n",result);
 	if (driver_byte(result) & DRIVER_SENSE) {
 		if (debug)
 			scsi_print_sense_hdr(ch->device, ch->name, &sshdr);
-- 
1.9.1

Re: [PATCH 3/4] ch: remove debug noise in ch_do_scsi

[Hannes Reinecke]

On 01/18/2015 04:11 PM, Christoph Hellwig wrote:
> The midlayer logging already prints the cdb details if the logging
> level is high enough, no need to duplicate this in the ch driver.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ch: remove debug noise in ch_do_scsi

[Ewan Milne]

On Sun, 2015-01-18 at 16:11 +0100, Christoph Hellwig wrote:
> The midlayer logging already prints the cdb details if the logging
> level is high enough, no need to duplicate this in the ch driver.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/ch.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
> index 79e462f..0045742 100644
> --- a/drivers/scsi/ch.c
> +++ b/drivers/scsi/ch.c
> @@ -194,18 +194,10 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd, int cmd_len,
>  
>   retry:
>  	errno = 0;
> -	if (debug) {
> -		char logbuf[SCSI_LOG_BUFSIZE];
> -
> -		__scsi_format_command(logbuf, sizeof(logbuf), cmd, cmd_len);
> -		DPRINTK("command: %s", logbuf);
> -	}
> -
>  	result = scsi_execute_req(ch->device, cmd, direction, buffer,
>  				  buflength, &sshdr, timeout * HZ,
>  				  MAX_RETRIES, NULL);
>  
> -	DPRINTK("result: 0x%x\n",result);
>  	if (driver_byte(result) & DRIVER_SENSE) {
>  		if (debug)
>  			scsi_print_sense_hdr(ch->device, ch->name, &sshdr);

Reviewed-by: Ewan D. Milne <emilne@redhat.com>

[PATCH 4/4] sr: reduce debug noise in sr_do_ioctl

[Christoph Hellwig]

The midlayer scsi logging already logs the command and sense code
if the logging level is high enough, no need to duplicate that
in the sr driver.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/sr_ioctl.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index e8deb9c..03054c0 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -188,7 +188,6 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
 	struct scsi_sense_hdr sshdr;
 	int result, err = 0, retries = 0;
 	struct request_sense *sense = cgc->sense;
-	char logbuf[SCSI_LOG_BUFSIZE];
 
 	SDev = cd->device;
 
@@ -246,9 +245,6 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
 				sr_printk(KERN_INFO, cd,
 					  "CDROM not ready.  Make sure there "
 					  "is a disc in the drive.\n");
-#ifdef DEBUG
-			scsi_print_sense_hdr(cd->device, cd->cdi.name, &sshdr);
-#endif
 			err = -ENOMEDIUM;
 			break;
 		case ILLEGAL_REQUEST:
@@ -257,22 +253,8 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
 			    sshdr.ascq == 0x00)
 				/* sense: Invalid command operation code */
 				err = -EDRIVE_CANT_DO_THIS;
-#ifdef DEBUG
-			__scsi_format_command(logbuf, sizeof(logbuf),
-					      cgc->cmd, CDROM_PACKET_SIZE);
-			sr_printk(KERN_INFO, cd,
-				  "CDROM (ioctl) invalid command: %s\n",
-				  logbuf);
-			scsi_print_sense_hdr(cd->device, cd->cdi.name, &sshdr);
-#endif
 			break;
 		default:
-			__scsi_format_command(logbuf, sizeof(logbuf),
-					      cgc->cmd, CDROM_PACKET_SIZE);
-			sr_printk(KERN_ERR, cd,
-				  "CDROM (ioctl) error, command: %s\n",
-				  logbuf);
-			scsi_print_sense_hdr(cd->device, cd->cdi.name, &sshdr);
 			err = -EIO;
 		}
 	}
-- 
1.9.1

Re: [PATCH 4/4] sr: reduce debug noise in sr_do_ioctl

[Hannes Reinecke]

On 01/18/2015 04:11 PM, Christoph Hellwig wrote:
> The midlayer scsi logging already logs the command and sense code
> if the logging level is high enough, no need to duplicate that
> in the sr driver.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] sr: reduce debug noise in sr_do_ioctl

[Ewan Milne]

On Sun, 2015-01-18 at 16:11 +0100, Christoph Hellwig wrote:
> The midlayer scsi logging already logs the command and sense code
> if the logging level is high enough, no need to duplicate that
> in the sr driver.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/sr_ioctl.c | 18 ------------------
>  1 file changed, 18 deletions(-)
> 
> diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
> index e8deb9c..03054c0 100644
> --- a/drivers/scsi/sr_ioctl.c
> +++ b/drivers/scsi/sr_ioctl.c
> @@ -188,7 +188,6 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
>  	struct scsi_sense_hdr sshdr;
>  	int result, err = 0, retries = 0;
>  	struct request_sense *sense = cgc->sense;
> -	char logbuf[SCSI_LOG_BUFSIZE];
>  
>  	SDev = cd->device;
>  
> @@ -246,9 +245,6 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
>  				sr_printk(KERN_INFO, cd,
>  					  "CDROM not ready.  Make sure there "
>  					  "is a disc in the drive.\n");
> -#ifdef DEBUG
> -			scsi_print_sense_hdr(cd->device, cd->cdi.name, &sshdr);
> -#endif
>  			err = -ENOMEDIUM;
>  			break;
>  		case ILLEGAL_REQUEST:
> @@ -257,22 +253,8 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
>  			    sshdr.ascq == 0x00)
>  				/* sense: Invalid command operation code */
>  				err = -EDRIVE_CANT_DO_THIS;
> -#ifdef DEBUG
> -			__scsi_format_command(logbuf, sizeof(logbuf),
> -					      cgc->cmd, CDROM_PACKET_SIZE);
> -			sr_printk(KERN_INFO, cd,
> -				  "CDROM (ioctl) invalid command: %s\n",
> -				  logbuf);
> -			scsi_print_sense_hdr(cd->device, cd->cdi.name, &sshdr);
> -#endif
>  			break;
>  		default:
> -			__scsi_format_command(logbuf, sizeof(logbuf),
> -					      cgc->cmd, CDROM_PACKET_SIZE);
> -			sr_printk(KERN_ERR, cd,
> -				  "CDROM (ioctl) error, command: %s\n",
> -				  logbuf);
> -			scsi_print_sense_hdr(cd->device, cd->cdi.name, &sshdr);
>  			err = -EIO;
>  		}
>  	}

Reviewed-by: Ewan D. Milne <emilne@redhat.com>

Re: [PATCH 1/4] scsi: always use format argumets for dev_printk

[James Bottomley]

On Sun, 2015-01-18 at 16:11 +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Thanks.  Ideally I would have preferred them folded into the logging
series, but if that's not going to be forthcoming, then this works.

For the series:

Reviewed-by: James Bottomley <JBottomley@Parallels.com>

James

Re: [PATCH 1/4] scsi: always use format argumets for dev_printk

[Hannes Reinecke]

On 01/18/2015 04:11 PM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] scsi: always use format argumets for dev_printk

[Ewan Milne]

On Sun, 2015-01-18 at 16:11 +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/scsi_logging.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c
> index 6128303..ecc5918 100644
> --- a/drivers/scsi/scsi_logging.c
> +++ b/drivers/scsi/scsi_logging.c
> @@ -251,7 +251,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
>  	if (cmd->cmd_len > 16) {
>  		/* Print opcode in one line and use separate lines for CDB */
>  		off += scnprintf(logbuf + off, logbuf_len - off, "\n");
> -		dev_printk(KERN_INFO, &cmd->device->sdev_gendev, logbuf);
> +		dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf);
>  		scsi_log_release_buffer(logbuf);
>  		for (k = 0; k < cmd->cmd_len; k += 16) {
>  			size_t linelen = min(cmd->cmd_len - k, 16);
> @@ -269,7 +269,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
>  						   16, 1, logbuf + off,
>  						   logbuf_len - off, false);
>  			}
> -			dev_printk(KERN_INFO, &cmd->device->sdev_gendev,
> +			dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s",
>  				   logbuf);
>  			scsi_log_release_buffer(logbuf);
>  		}
> @@ -282,7 +282,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
>  				   false);
>  	}
>  out_printk:
> -	dev_printk(KERN_INFO, &cmd->device->sdev_gendev, logbuf);
> +	dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf);
>  	scsi_log_release_buffer(logbuf);
>  }
>  EXPORT_SYMBOL(scsi_print_command);
> @@ -360,7 +360,7 @@ scsi_log_dump_sense(const struct scsi_device *sdev, const char *name, int tag,
>  		hex_dump_to_buffer(&sense_buffer[i], len, 16, 1,
>  				   logbuf + off, logbuf_len - off,
>  				   false);
> -		dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
> +		dev_printk(KERN_INFO, &sdev->sdev_gendev, "%s", logbuf);
>  	}
>  	scsi_log_release_buffer(logbuf);
>  }
> @@ -377,7 +377,7 @@ scsi_log_print_sense_hdr(const struct scsi_device *sdev, const char *name,
>  		return;
>  	off = sdev_format_header(logbuf, logbuf_len, name, tag);
>  	off += scsi_format_sense_hdr(logbuf + off, logbuf_len - off, sshdr);
> -	dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
> +	dev_printk(KERN_INFO, &sdev->sdev_gendev, "%s", logbuf);
>  	scsi_log_release_buffer(logbuf);
>  
>  	logbuf = scsi_log_reserve_buffer(&logbuf_len);
> @@ -386,7 +386,7 @@ scsi_log_print_sense_hdr(const struct scsi_device *sdev, const char *name,
>  	off = sdev_format_header(logbuf, logbuf_len, name, tag);
>  	off += scsi_format_extd_sense(logbuf + off, logbuf_len - off,
>  				      sshdr->asc, sshdr->ascq);
> -	dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
> +	dev_printk(KERN_INFO, &sdev->sdev_gendev, "%s", logbuf);
>  	scsi_log_release_buffer(logbuf);
>  }
>  
> @@ -483,7 +483,7 @@ void scsi_print_result(const struct scsi_cmnd *cmd, const char *msg,
>  		off += scnprintf(logbuf + off, logbuf_len - off,
>  				 "driverbyte=0x%02x", driver_byte(cmd->result));
>  out_printk:
> -	dev_printk(KERN_INFO, &cmd->device->sdev_gendev, logbuf);
> +	dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf);
>  	scsi_log_release_buffer(logbuf);
>  }
>  EXPORT_SYMBOL(scsi_print_result);

Reviewed-by: Ewan D. Milne <emilne@redhat.com>