On Sun, Sep 23, 2007 at 09:08:46AM -0500, James Bottomley wrote:
> The main thing I don't like is that we've spent a lot of time moving
> sense prints into a library so that if someone gullible^Wsensible enough
> to want to do a reporting infrastructure comes along, we have all the
> entry points nicely laid out, this is a retrograde step on that because
> it reintroduces the open coding.

Your patch is certainly an improvement from that point of view; it
consolidates two calls (which would have to be tied together by said
infrastructure) into one.

> How about something like the attached.  It does exactly what you want
> but retains the tap into the constants file for logging?

> +/*
> + * Print normalized SCSI sense header with device information and a prefix.
> + */
> +void
> +scsi_cmd_print_sense_hdr(struct scsi_cmnd *scmd, const char *name,
> +                       struct scsi_sense_hdr *sshdr)
> +{
> +     scmd_printk(KERN_INFO, scmd, "%s: ", name);
> +     scsi_show_sense_hdr(sshdr);
> +     scmd_printk(KERN_INFO, scmd, "%s: ", name);
> +     scsi_show_extd_sense(sshdr->asc, sshdr->ascq);
> +}

I think 'name' is wrong.  It's not a name, it's a description.  How
about s/name/desc/g ?

With that change, Acked-by: Matthew Wilcox <[EMAIL PROTECTED]>

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to