[Spice-devel] [PATCH V2] usbredir: fix redirection of user-accesible device nodes.

[Christophe Fergeau]

On Fri, Jul 10, 2015 at 10:18:15AM +0200, Michal Suchanek wrote:
> Excerpts from Christophe Fergeau's message of Thu Jul 09 17:11:45 +0200 2015:
> > On Thu, Jul 09, 2015 at 04:58:52PM +0200, Michal Suchanek wrote:
> > > Excerpts from Christophe Fergeau's message of Thu Jul 09 16:41:58 +0200 2015:
> > > > > -    spice_usb_acl_helper_open_acl_finish(acl_helper, acl_res, &err);
> > > > > -    if (!err && priv->state == STATE_DISCONNECTING) {
> > > > > +    spice_usb_acl_helper_open_acl_finish(acl_helper, acl_res, &acl_err);
> > > > > +    if (!acl_err && priv->state == STATE_DISCONNECTING) {
> > > > 
> > > > Here I'm wondering if you should not drop the if (!acl_err) part.
> > > 
> > > That's the original code. It's probably meant to show the cancelling
> > > error only if no other error happened so far.
> > > 
> > > Either way if acl_err is set it will be shown regardless of err.
> > 
> > The initial code reads "if state is DISCONNECTING, then set 'err' if
> > it's not set already" that is, "if _open_acl_finish() returned an error,
> > or if state is DISCONNECTING, then 'err' is set, and we will report an
> > error"
> > After your changes, this has become "if _open_acl_finish() did not return an
> > error and state is DISCONNECTING, then 'err' is set, annd we will
> > rerport an error". In particular, in the case when '_open_acl_finish() returned an
> > error, and open_device() succeeded, no error will be returned'. In my
> > opinion, it makes more sense as "if state is _DISCONNECTING regardless
> > of acl_error, then return an error".
> > 
> > (let me know if I'm not making myself clear)
> > 
> > Christophe
> 
> And that's discussed in the latter part of the previous email you
> trimmed. I missed this case when err is set by hand and removed the
> if (err) part which skips the cancellation check.

No I think this is something different, think about acl_err being set,
priv->state == STATE_DISCONNECTING, and _open_device() succeeding (so
err is not set).

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150710/7916845a/attachment.sig>