On Thu, Aug 28, 2014 at 07:33:20PM +0200, Hannes Reinecke wrote:
> If scsi_normalize_sense() fails we couldn't decode the sense
> buffer, so we should not try to interpret the result.
> We should rather dump the sense buffer instead and stop decoding.

as far as I can tell the old code doesn't interpret it, it just does a
slightly more convoluted hexdump than your version.  Care to come up
with a better description for this patch?

>  static void
> +scsi_dump_sense_buffer(struct scsi_device *sdev, const char *name,
> +                    const unsigned char *sense_buffer, int sense_len)
>  {
> +     char linebuf[128];
> +     int i, linelen, remaining;
>  
> +     remaining = sense_len;
> +     for (i = 0; i < sense_len; i += 16) {
> +             linelen = min(remaining, 16);
> +             remaining -= 16;
> +
> +             hex_dump_to_buffer(sense_buffer + i, linelen, 16, 1,
> +                                linebuf, sizeof(linebuf), false);
> +             sdev_prefix_printk(KERN_INFO, sdev, name,
> +                                "Sense: %s\n", linebuf);
>       }

This would be more readanle without the remaining variable:

        for (i = 0; i < sense_len; i += 16) {
                linelen = min(senselen - i, 16);
        
                ...
        }

> @@ -1517,8 +1513,13 @@ void __scsi_print_sense(struct scsi_device *sdev, 
> const char *name,
>                       const unsigned char *sense_buffer, int sense_len)
>  {
>       struct scsi_sense_hdr sshdr;
> +     int res;
>  
> -     scsi_decode_sense_buffer(sense_buffer, sense_len, &sshdr);
> +     res = scsi_normalize_sense(sense_buffer, sense_len, &sshdr);
> +     if (res == 0) {
> +             scsi_dump_sense_buffer(sdev, name, sense_buffer, sense_len);
> +             return;
> +     }

no need for the res variable.  In fact due to the boolean-like return
value of scsi_normalize_sense it would be a lot more readable without
the variable.

--
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