Oliver Neukum <oli...@neukum.org> writes:

> On Tuesday 26 February 2013 14:59:23 Bjørn Mork wrote:
>> Oliver Neukum <oli...@neukum.org> writes:
>> 
>> > @@ -185,9 +187,17 @@ static void wdm_in_callback(struct urb *urb)
>> >    }
>> >  
>> >    desc->rerr = status;
>> > -  desc->reslength = urb->actual_length;
>> > -  memmove(desc->ubuf + desc->length, desc->inbuf, desc->reslength);
>> > -  desc->length += desc->reslength;
>> > +  if (length + desc->length > desc->wMaxCommand) {
>> > +          /* The buffer would overflow */
>> > +          set_bit(WDM_OVERFLOW, &desc->flags);
>> > +  } else {
>> > +          /* we may already be in overflow */
>> > +          if (!test_bit(WDM_OVERFLOW, &desc->flags)) {
>> > +                  memmove(desc->ubuf + desc->length, desc->inbuf, 
>> > desc->reslength);
>> > +                  desc->length += length;
>> > +                  desc->reslength = length;
>> > +          }
>> 
>> 
>> This is not OK.  Should probably be
>
> Thank you!
> The tests failed and I couldn't find out why. Sometimes I am blind.

You're welcome. More than 2 eyes always help :)

I believe we could drop the reslength field altogether, avoiding this
confusion. The field is only use to test for a 0 length read in a
context where desc->length is known to be 0.  In fact, we are really
interested in the total desc->length anyway.  The length of the last
read is completely irrelevant.

If we don't drop it, then I believe you should set it to 0 on overflow.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to