Excerpts from Christophe Fergeau's message of Thu Jul 09 13:47:51 +0200 2015:
> On Mon, Jun 29, 2015 at 03:46:56PM +0200, 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>
> > ---
> > src/channel-usbredir.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> > index 292b82f..b5745b6 100644
> > --- a/src/channel-usbredir.c
> > +++ b/src/channel-usbredir.c
> > @@ -276,21 +276,24 @@ 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);
>
> I assume in your setup, 'acl_err' gets set when calling open_acl_finish?
> What is the error message you are getting? (ideally error domain/code
> would be nice too).
I have no policykit set up so every attempt to open anything is denied
due to insufficient permissions. I can get the exact error if it's
important.
>
> > + if (!acl_err && priv->state == STATE_DISCONNECTING) {
> > err = g_error_new_literal(G_IO_ERROR, G_IO_ERROR_CANCELLED,
> > "USB redirection channel connect cancelled");
> > }
> > - if (!err) {
> > - spice_usbredir_channel_open_device(channel, &err);
> > - }
> > +
> > + spice_usbredir_channel_open_device(channel, &err);
>
> given that you added separate acl_err and err, you can keep the if
> (!err) { } unchanged here I think
err is NULL here so it's nop either way.
>
> > if (err) {
>
> but here this will need to be if (err || acl_err) {
No. If err is NULL then open succeeded. Err is set by actually opening
the device.
>
> > - g_simple_async_result_take_error(priv->result, err);
> > + if (acl_err)
> > + g_simple_async_result_take_error(priv->result, acl_err);
> > + else
> > + g_simple_async_result_take_error(priv->result, err);
>
> If open_device() failed after open_acl_finish() failing, both acl_err
> and err will be set, so you need to free the one you are not going to
> use.
Which is the case when calling out to the policykit does not give
permission to open and the open actually fails.
> I'd do
> if (acl_err) {
> g_simple_async_result_take_error(priv->result, acl_err);
> acl_err = NULL;
> } else {
> g_simple_async_result_take_error(priv->result, err);
> err = NULL;
> }
> g_clear_error(acl_err);
> g_clear_error(err);
If that's the way to handle the errors it's easy to add.
Thanks
Michal
>
>
>
> Christophe
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel