Re: [PATCH] usb: gadget: f_fs: report error if excess data received

[Michal Nazarewicz]

On Tue, May 17 2016, Changbin Du wrote:
>> There appears to be no kfifo support for iov_iter though, so I just went
>> with a simple buffer.
>> 
>> I havenât looked at the patch too carefully so this is an RFC rather
>> than an actual patch at this point.  It does compile at least.
>> 
>> Regardless, the more I thin about it, the more Iâm under the impression
>> that the whole rounding up in f_fs was a mistake.  And the more Iâm
>> leaning towards ignoring the excess data set by the host.
>> 
>> ---------- >8 ----------------------------------------------------------
>> Subject: usb: gadget: f_fs: buffer data from âoversizedâ OUT requests
>> 
>> f_fs rounds up read(2) requests to a multiple of a max packet size
>> which means that host may provide more data than user has space for.
>> So far, the excess data has been silently ignored.
>> 
>> This introduces a buffer for a tail of such requests so that they are
>> returned on next read instead of being ignored.
>> 
>
> Congratulations finally reach an agreement,

To be honest, if it was up to me, I would rip request size rounding up
out of the code.

> thanks Alan Stern and Michal.
> Here just have a comment - the buffered data need be dropped when the
> epfile is closed, because it means the session is terminated.

I blame that on sleep deprivation.  Another issue is what to do when
endpoint is disabled.  Should the buffer be cleared as soon as the
endpoint is disabled?  Or maybe when the endpoint is enabled again?  Or
maybe it should never be cleared?

If the buffer is cleared when endpoint is disabled, we again silently
drop data.  On the other hand, if we donât do that, read() on the
endpoint will may succeed even if the configuration is disabled which
may be surprising for users.

-- 
Best regards
ããã âðððð86â ãããããã
ÂIf at first you donât succeed, give up skydivingÂ