* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.