Oliver Neukum <oliver@xxxxxxxxxx> writes:
> On Tuesday 26 February 2013 14:59:23 Bjørn Mork wrote:
>> Oliver Neukum <oliver@xxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
