* [PATCH 0/2] libosd: bug fixes @ 2009-11-08 12:27 Boaz Harrosh 2009-11-08 12:29 ` [PATCH 1/2] libosd: bug in osd_req_decode_sense_full() Boaz Harrosh 2009-11-08 12:30 ` [PATCH 2/2] libosd: Bugfix of error handling in attributes-list decoding Boaz Harrosh 0 siblings, 2 replies; 5+ messages in thread From: Boaz Harrosh @ 2009-11-08 12:27 UTC (permalink / raw) To: James Bottomley, linux-scsi, open-osd These are two additional bug-fix patches to libosd for the 2.6.33 merge window. Please consider for submission on top of the previous 5 [PATCH 1/2] libosd: bug in osd_req_decode_sense_full() [PATCH 2/2] libosd: Bugfix of error handling in attributes-list decoding Thanks Boaz ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] libosd: bug in osd_req_decode_sense_full() 2009-11-08 12:27 [PATCH 0/2] libosd: bug fixes Boaz Harrosh @ 2009-11-08 12:29 ` Boaz Harrosh 2009-11-10 15:09 ` [osd-dev] " Boaz Harrosh 2009-11-10 15:10 ` [PATCH1/2 version2] " Boaz Harrosh 2009-11-08 12:30 ` [PATCH 2/2] libosd: Bugfix of error handling in attributes-list decoding Boaz Harrosh 1 sibling, 2 replies; 5+ messages in thread From: Boaz Harrosh @ 2009-11-08 12:29 UTC (permalink / raw) To: James Bottomley, linux-scsi, open-osd The (never tested) osd_sense_attribute_identification case has never worked. The loop was never advanced on. Fix it to work as intended. On 10/30/2009 04:39 PM, Roel Kluin wrote: I found this by code analysis, searching for while loops that test a local variable, but do not modify the variable. Reported-by: Roel Kluin <roel.kluin@gmail.com> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> --- drivers/scsi/osd/osd_initiator.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c index 60b7ca1..d504e82 100644 --- a/drivers/scsi/osd/osd_initiator.c +++ b/drivers/scsi/osd/osd_initiator.c @@ -1584,14 +1584,13 @@ int osd_req_decode_sense_full(struct osd_request *or, struct osd_sense_attributes_data_descriptor *osadd = cur_descriptor; int len = min(cur_len, sense_len); - int i = 0; struct osd_sense_attr *pattr = osadd->sense_attrs; - while (len < 0) { + while (len >= sizeof(*pattr)) { u32 attr_page = be32_to_cpu(pattr->attr_page); u32 attr_id = be32_to_cpu(pattr->attr_id); - if (i++ == 0) { + if (!osi->attr.attr_page) { osi->attr.attr_page = attr_page; osi->attr.attr_id = attr_id; } @@ -1602,6 +1601,8 @@ int osd_req_decode_sense_full(struct osd_request *or, bad_attr_list++; max_attr--; } + + len -= sizeof(*pattr); OSD_SENSE_PRINT2( "osd_sense_attribute_identification" "attr_page=0x%x attr_id=0x%x\n", -- 1.6.5.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [osd-dev] [PATCH 1/2] libosd: bug in osd_req_decode_sense_full() 2009-11-08 12:29 ` [PATCH 1/2] libosd: bug in osd_req_decode_sense_full() Boaz Harrosh @ 2009-11-10 15:09 ` Boaz Harrosh 2009-11-10 15:10 ` [PATCH1/2 version2] " Boaz Harrosh 1 sibling, 0 replies; 5+ messages in thread From: Boaz Harrosh @ 2009-11-10 15:09 UTC (permalink / raw) To: James Bottomley, linux-scsi, open-osd On 11/08/2009 02:29 PM, Boaz Harrosh wrote: > > The (never tested) osd_sense_attribute_identification case > has never worked. The loop was never advanced on. > Fix it to work as intended. > > On 10/30/2009 04:39 PM, Roel Kluin wrote: > I found this by code analysis, searching for while > loops that test a local variable, but do not modify > the variable. > > Reported-by: Roel Kluin <roel.kluin@gmail.com> > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> > --- > drivers/scsi/osd/osd_initiator.c | 7 ++++--- > 1 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c > index 60b7ca1..d504e82 100644 > --- a/drivers/scsi/osd/osd_initiator.c > +++ b/drivers/scsi/osd/osd_initiator.c > @@ -1584,14 +1584,13 @@ int osd_req_decode_sense_full(struct osd_request *or, > struct osd_sense_attributes_data_descriptor > *osadd = cur_descriptor; > int len = min(cur_len, sense_len); len should be unsigned for the compare with sizeof() below. I'll repost a second version as reply. Boaz > - int i = 0; > struct osd_sense_attr *pattr = osadd->sense_attrs; > > - while (len < 0) { > + while (len >= sizeof(*pattr)) { > u32 attr_page = be32_to_cpu(pattr->attr_page); > u32 attr_id = be32_to_cpu(pattr->attr_id); > > - if (i++ == 0) { > + if (!osi->attr.attr_page) { > osi->attr.attr_page = attr_page; > osi->attr.attr_id = attr_id; > } > @@ -1602,6 +1601,8 @@ int osd_req_decode_sense_full(struct osd_request *or, > bad_attr_list++; > max_attr--; > } > + > + len -= sizeof(*pattr); > OSD_SENSE_PRINT2( > "osd_sense_attribute_identification" > "attr_page=0x%x attr_id=0x%x\n", ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH1/2 version2] libosd: bug in osd_req_decode_sense_full() 2009-11-08 12:29 ` [PATCH 1/2] libosd: bug in osd_req_decode_sense_full() Boaz Harrosh 2009-11-10 15:09 ` [osd-dev] " Boaz Harrosh @ 2009-11-10 15:10 ` Boaz Harrosh 1 sibling, 0 replies; 5+ messages in thread From: Boaz Harrosh @ 2009-11-10 15:10 UTC (permalink / raw) To: James Bottomley, linux-scsi, open-osd The (never tested) osd_sense_attribute_identification case has never worked. The loop was never advanced on. Fix it to work as intended. On 10/30/2009 04:39 PM, Roel Kluin wrote: I found this by code analysis, searching for while loops that test a local variable, but do not modify the variable. Reported-by: Roel Kluin <roel.kluin@gmail.com> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> --- drivers/scsi/osd/osd_initiator.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c index 60b7ca1..5e90d19 100644 --- a/drivers/scsi/osd/osd_initiator.c +++ b/drivers/scsi/osd/osd_initiator.c @@ -1583,15 +1583,14 @@ int osd_req_decode_sense_full(struct osd_request *or, { struct osd_sense_attributes_data_descriptor *osadd = cur_descriptor; - int len = min(cur_len, sense_len); - int i = 0; + unsigned len = min(cur_len, sense_len); struct osd_sense_attr *pattr = osadd->sense_attrs; - while (len < 0) { + while (len >= sizeof(*pattr)) { u32 attr_page = be32_to_cpu(pattr->attr_page); u32 attr_id = be32_to_cpu(pattr->attr_id); - if (i++ == 0) { + if (!osi->attr.attr_page) { osi->attr.attr_page = attr_page; osi->attr.attr_id = attr_id; } @@ -1602,6 +1601,8 @@ int osd_req_decode_sense_full(struct osd_request *or, bad_attr_list++; max_attr--; } + + len -= sizeof(*pattr); OSD_SENSE_PRINT2( "osd_sense_attribute_identification" "attr_page=0x%x attr_id=0x%x\n", -- 1.6.5.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] libosd: Bugfix of error handling in attributes-list decoding 2009-11-08 12:27 [PATCH 0/2] libosd: bug fixes Boaz Harrosh 2009-11-08 12:29 ` [PATCH 1/2] libosd: bug in osd_req_decode_sense_full() Boaz Harrosh @ 2009-11-08 12:30 ` Boaz Harrosh 1 sibling, 0 replies; 5+ messages in thread From: Boaz Harrosh @ 2009-11-08 12:30 UTC (permalink / raw) To: James Bottomley, linux-scsi, open-osd When an error was detected in an attribute list, do to a target bug. We would print an error but spin endlessly regardless. Fix it. Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> --- drivers/scsi/osd/osd_initiator.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c index d504e82..26d8dc0 100644 --- a/drivers/scsi/osd/osd_initiator.c +++ b/drivers/scsi/osd/osd_initiator.c @@ -1167,6 +1167,7 @@ int osd_req_decode_get_attr_list(struct osd_request *or, "c=%d r=%d n=%d\n", cur_bytes, returned_bytes, n); oa->val_ptr = NULL; + cur_bytes = returned_bytes; /* break the caller loop */ break; } -- 1.6.5.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-11-10 15:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-11-08 12:27 [PATCH 0/2] libosd: bug fixes Boaz Harrosh 2009-11-08 12:29 ` [PATCH 1/2] libosd: bug in osd_req_decode_sense_full() Boaz Harrosh 2009-11-10 15:09 ` [osd-dev] " Boaz Harrosh 2009-11-10 15:10 ` [PATCH1/2 version2] " Boaz Harrosh 2009-11-08 12:30 ` [PATCH 2/2] libosd: Bugfix of error handling in attributes-list decoding Boaz Harrosh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).