On Mon, Dec 10, 2018 at 08:25:02PM +0000, Jason Gunthorpe wrote: > On Mon, Dec 10, 2018 at 10:16:09PM +0200, Leon Romanovsky wrote: > > On Mon, Dec 10, 2018 at 07:00:12PM +0000, Jason Gunthorpe wrote: > > > On Mon, Dec 10, 2018 at 11:35:34AM +0200, Leon Romanovsky wrote: > > > > + if (resp_len < attrs->ucore.outlen) > > > > + /* > > > > + * Zero fill any extra memory that user > > > > + * space might have provided. > > > > + */ > > > > + ret = clear_user(attrs->ucore.outbuf + resp_len, > > > > + attrs->ucore.outlen - resp_len); > > > > > > > > - return 0; > > > > + return (ret) ? -EFAULT : 0; > > > > > > This would be nicer to stay as return 0 so we don't have to have extra > > > branches on the fast path. Code is clearer that way too.. > > > > I don't see usage of this function in "fast path", only in our standard > > control verbs which are far to be optimized for the speed. > > It is fast path for some drivers and some use-cases, this is why we go > to such lengths in some cases. > > > Let's try to avoid premature optimization and I disagree with second > > part of the question too :) > > If there is an error, then just detect it and return, or exit with > goto. > > Don't try and carry the error through control flow tricks, it is ugly > and fragile clear_user doesn't return errno, so some assignment is still needed. I'll resend. Thanks > > Jason
Attachment:
signature.asc
Description: PGP signature
