On 09/01/2014 12:00 AM, Christoph Hellwig wrote: > 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? > Yep, done. >> 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); > > ... > } > Ok, fixed. >> @@ -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. > Ok, fixed. 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