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:
> > > On Thu, Jul 09, 2015 at 03:25:19PM +0100, Michal Suchanek wrote:
> > > > When calling ACL helper fails also try to open the device node directly.
> > > >
> > > > Otherwise user-accessible device nodes are rejected when policykit
> > > > support is compiled in and policy is not set up when in fact the device
> > > > could be accessed.
> > > >
> > > > Signed-off-by: Michal Suchanek <hramrach@xxxxxxxxx>
> > > >
> > > > --
> > > >
> > > > v2:
> > > >
> > > > - clear errors properly
> > > > ---
> > > > src/channel-usbredir.c | 20 ++++++++++++++------
> > > > 1 file changed, 14 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> > > > index 292b82f..7f4ddd7 100644
> > > > --- a/src/channel-usbredir.c
> > > > +++ b/src/channel-usbredir.c
> > > > @@ -276,27 +276,35 @@ static void spice_usbredir_channel_open_acl_cb(
> > > > SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(user_data);
> > > > SpiceUsbredirChannelPrivate *priv = channel->priv;
> > > > GError *err = NULL;
> > > > + GError *acl_err = NULL;
> > > >
> > > > g_return_if_fail(acl_helper == priv->acl_helper);
> > > > g_return_if_fail(priv->state == STATE_WAITING_FOR_ACL_HELPER ||
> > > > priv->state == STATE_DISCONNECTING);
> > > >
> > > > - 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.
Thanks
Michal
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel