On Thu, 2015-01-08 at 07:43 +0100, Hannes Reinecke wrote:
> Use an external buffer for __scsi_print_command() and
> move command logging over to use the per-cpu logging
> buffer. With that we can guarantee the command always
> will always be formatted in one line.
> So we can even print out a variable length command
> correctly across several lines.
> Finally rename __scsi_print_command() to
> __scsi_format_comment() to better reflect the
> functionality.
>
> Tested-by: Robert Elliott <elliott@xxxxxx>
> Reviewed-by: Robert Elliott <elliott@xxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
> ---
> drivers/scsi/ch.c | 6 +-
> drivers/scsi/constants.c | 74 +-----------------
> drivers/scsi/scsi_logging.c | 182 ++++++++++++++++++++++++++++++++++++++++----
> drivers/scsi/sr_ioctl.c | 13 +++-
> include/scsi/scsi.h | 3 +
> include/scsi/scsi_dbg.h | 6 +-
> 6 files changed, 192 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
> index 6bac8a7..79e462f 100644
> --- a/drivers/scsi/ch.c
> +++ b/drivers/scsi/ch.c
> @@ -195,8 +195,10 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd, int cmd_len,
> retry:
> errno = 0;
> if (debug) {
> - DPRINTK("command: ");
> - __scsi_print_command(cmd, cmd_len);
> + char logbuf[SCSI_LOG_BUFSIZE];
> +
> + __scsi_format_command(logbuf, sizeof(logbuf), cmd, cmd_len);
> + DPRINTK("command: %s", logbuf);
I really hate using an on-stack buffer here ... we're pretty deep in the
call chain already.
Since it's just required for printing a "command: " prefix, why not just
use scsi_print_command()?
> diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
> index fb929fa..e8deb9c 100644
> --- a/drivers/scsi/sr_ioctl.c
> +++ b/drivers/scsi/sr_ioctl.c
> @@ -188,6 +188,7 @@ 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];
Same issue here.
> SDev = cd->device;
>
> @@ -257,14 +258,20 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
> /* sense: Invalid command operation code */
> err = -EDRIVE_CANT_DO_THIS;
> #ifdef DEBUG
> - __scsi_print_command(cgc->cmd, CDROM_PACKET_SIZE);
> + __scsi_format_command(logbuf, sizeof(logbuf),
> + cgc->cmd, CDROM_PACKET_SIZE);
> + sr_printk(KERN_INFO, cd,
> + "CDROM (ioctl) invalid command: %s\n",
> + logbuf);
Looks like scsi_print_command() will work here too.
> 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: ");
> - __scsi_print_command(cgc->cmd, CDROM_PACKET_SIZE);
> + "CDROM (ioctl) error, command: %s\n",
> + logbuf);
And realistically here (with the appropriate sdev_print()'s preceding).
> scsi_print_sense_hdr(cd->device, cd->cdi.name, &sshdr);
> err = -EIO;
> }
James
��.n��������+%������w��{.n�����{������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
