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
Attachment:
pgp7EnAoPHnM6.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel