Oliver Neukum <oneukum@xxxxxxxx> writes:
> Am Donnerstag, den 20.04.2017, 10:32 +0200 schrieb Bjørn Mork:
>> > @@ -189,7 +192,13 @@ static void wdm_in_callback(struct urb *urb)
>> > }
>> > }
>> >
>> > - desc->rerr = status;
>> > + /*
>> > + * only set a new error if there is no previous error.
>> > + * Errors are only cleared during read/open
>> > + */
>> > + if (desc->rerr == 0)
>> > + desc->rerr = status;
>> > +
>> > if (length + desc->length > desc->wMaxCommand) {
>> > /* The buffer would overflow */
>> > set_bit(WDM_OVERFLOW, &desc->flags);
>> >
>>
>>
>> This is minor, but in my opinion this change is flawed. It changes the
>> meaning of "desc->rerr" from the original "last status code" to the new
>> "first error since last userspace read".
>
> Indeed. And that will preserve the initial cause of the problem better.
> We could run into the zero length read case. Are you sure you want the
> old behavior back?
No, I am not sure. But I don't see how we can both cache the error and
continue to read new data. The error is irrelevant after the next URB
has completed. If we decide that every error should be delivered to
userspace, then we either have to stop transmission or maintain a queue
of device reads, keeping data and errors in sync. Delivering an old
error to userspace along with newer data makes no sense.
Also note that this code path is skipped entirely for several common
errors. So they are never reported to userspace.
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
