Messages in this thread |  | | | Date | Wed, 03 Feb 2010 14:55:25 +0900 | | From | Kei Tokunaga <> | | Subject | Re: [PATCH 2/2 v2] scsi: add scsi trace core function and put trace points |
| |
Martin K. Petersen wrote: >>>>>> "Kei" == Kei Tokunaga <tokunaga.keiich@jp.fujitsu.com> writes: > > I'm traveling so I won't have time to look at this closely until next > week. However, this caught my eye: > > +static const char * > +scsi_trace_varlen(struct trace_seq *p, unsigned char *cdb, int len) > +{ > + switch (SERVICE_ACTION(cdb)) { > + case READ_32: > + case WRITE_32: > + /* if protection is enabled */ > + if (((cdb[10] >> 5) & 0x7) == 1) > + return scsi_trace_rw32(p, cdb, len); > + /* fall through */ > + default: > + return scsi_trace_misc(p, cdb, len); > + } > +} > > It is not a requirement that a 32-byte READ/WRITE request must have > PROTECT set. So that if statement is bogus.
Yes. I agree that the if statement is not necessary here. I'll fix it. Thanks for having your time for the review.
Kei
|  |