On Wed, 2011-09-28 at 14:04 -0700, Nicholas A. Bellinger wrote: > On Tue, 2011-09-27 at 13:48 +0300, Andy Shevchenko wrote: > > On Mon, Sep 19, 2011 at 11:11 AM, Andy Shevchenko > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > > Cc: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx> > > > --- > > > drivers/target/target_core_cdb.c | 30 +++++++++++++----------------- > > > 1 files changed, 13 insertions(+), 17 deletions(-) > > > > > > diff --git a/drivers/target/target_core_cdb.c b/drivers/target/target_core_cdb.c > > > index f04d4ef..b796115 100644 > > > --- a/drivers/target/target_core_cdb.c > > > +++ b/drivers/target/target_core_cdb.c > > > @@ -24,7 +24,6 @@ > > > */ > > > > > > #include <linux/kernel.h> > > > -#include <linux/ctype.h> > > > #include <asm/unaligned.h> > > > #include <scsi/scsi.h> > > > > > > @@ -156,11 +155,12 @@ target_emulate_evpd_80(struct se_cmd *cmd, unsigned char *buf) > > > } > > > > > > static void > > > -target_parse_naa_6h_vendor_specific(struct se_device *dev, unsigned char *buf_off) > > > +target_parse_naa_6h_vendor_specific(struct se_device *dev, unsigned char *buf) > > > { > > > unsigned char *p = &dev->se_sub_dev->t10_wwn.unit_serial[0]; > > > - unsigned char *buf = buf_off; > > > - int cnt = 0, next = 1; > > > + int cnt; > > > + bool next = true; > > > + > > > /* > > > * Generate up to 36 bits of VENDOR SPECIFIC IDENTIFIER starting on > > > * byte 3 bit 3-0 for NAA IEEE Registered Extended DESIGNATOR field > > > @@ -169,20 +169,16 @@ target_parse_naa_6h_vendor_specific(struct se_device *dev, unsigned char *buf_of > > > * NUMBER set via vpd_unit_serial in target_core_configfs.c to ensure > > > * per device uniqeness. > > > */ > > > - while (*p != '\0') { > > > - if (cnt >= 13) > > > - break; > > > - if (!isxdigit(*p)) { > > > - p++; > > > + for (cnt = 0; *p && cnt < 13; next = !next) { > > > + int val = hex_to_bin(*p++); > > > + > > > + if (val < 0) > > > continue; > > > - } > > > - if (next != 0) { > > > - buf[cnt++] |= hex_to_bin(*p++); > > > - next = 0; > > > - } else { > > > - buf[cnt] = hex_to_bin(*p++) << 4; > > > - next = 1; > > > - } > > > + > > > + if (next) > > > + buf[cnt++] |= val; > > > + else > > > + buf[cnt] = val << 4; > > > } > > > } > > > > > > > Nicholas, any comment on this? > > > > Hi Andy, > > This simplification is fine with me, and will plan to queue this for > v3.2. > Hi Andy, So I finally got around to testing this patch, but unfortunately it does not produce the same results as the original code. The problem is due to the 'next = !next' assignment in the above for loop, which causes the value to be reset even when hex_to_bin() detects an non hex character.. Here is an updated patch that I will be commiting into lio-core-2.6.git. Thanks, --nab diff --git a/drivers/target/target_core_cdb.c b/drivers/target/target_core_cdb.c index f04d4ef..0d02391 100644 --- a/drivers/target/target_core_cdb.c +++ b/drivers/target/target_core_cdb.c @@ -24,7 +24,6 @@ */ #include <linux/kernel.h> -#include <linux/ctype.h> #include <asm/unaligned.h> #include <scsi/scsi.h> @@ -156,11 +155,12 @@ target_emulate_evpd_80(struct se_cmd *cmd, unsigned char *buf) } static void -target_parse_naa_6h_vendor_specific(struct se_device *dev, unsigned char *buf_off) +target_parse_naa_6h_vendor_specific(struct se_device *dev, unsigned char *buf) { unsigned char *p = &dev->se_sub_dev->t10_wwn.unit_serial[0]; - unsigned char *buf = buf_off; - int cnt = 0, next = 1; + int cnt; + bool next = true; + /* * Generate up to 36 bits of VENDOR SPECIFIC IDENTIFIER starting on * byte 3 bit 3-0 for NAA IEEE Registered Extended DESIGNATOR field @@ -169,19 +169,18 @@ target_parse_naa_6h_vendor_specific(struct se_device *dev, unsigned char *buf_of * NUMBER set via vpd_unit_serial in target_core_configfs.c to ensure * per device uniqeness. */ - while (*p != '\0') { - if (cnt >= 13) - break; - if (!isxdigit(*p)) { - p++; + for (cnt = 0; *p && cnt < 13; p++) { + int val = hex_to_bin(*p); + + if (val < 0) continue; - } - if (next != 0) { - buf[cnt++] |= hex_to_bin(*p++); - next = 0; + + if (next) { + next = false; + buf[cnt++] |= val; } else { - buf[cnt] = hex_to_bin(*p++) << 4; - next = 1; + next = true; + buf[cnt] = val << 4; } } } -- 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