On Thu, Sep 27, 2018 at 11:48:13AM +0300, Yuri Benditovich wrote:
> > > -static int usbredir_read_callback(void *user_data, uint8_t *data, int
> > count)
> > > +static void usbredir_log(void *user_data, const char *msg, gboolean
> > error)
> > > {
> > > SpiceUsbredirChannel *channel = user_data;
> > > SpiceUsbredirChannelPrivate *priv = channel->priv;
> > >
> > > - count = MIN(priv->read_buf_size, count);
> > > -
> > > - if (count != 0) {
> > > - memcpy(data, priv->read_buf, count);
> > > + if (priv->catch_error && error) {
> > > + g_set_error_literal(priv->catch_error, SPICE_CLIENT_ERROR,
> > > + SPICE_CLIENT_ERROR_FAILED, msg);
> > > + priv->catch_error = NULL;
> >
> > It's odd to set priv->catch_error and set it to NULL right after setting
> > it.
> >
>
> It's correct to set it to NULL if used once.
> I'll add comment for that.
If you mean that we don't want to try to set priv->catch_error multiple
times, it probably would be clearer if you explicitly check for that:
if (error && !error_is_set(priv->catch_error)) {
g_set_error_literal(...);
}
with
bool error_is_set(GError **error)
{
return ((error != NULL) && *error != NULL));
}
> > > /**
> > > * spice_usb_device_get_libusb_device:
> > > - * @device: #SpiceUsbDevice to get the descriptor information of
> > > + * @device: #SpiceUsbDevice to get the libusb device of (if exists)
> > > *
> > > * Finds the %libusb_device associated with the @device.
> > > *
> > > - * Returns: (transfer none): the %libusb_device associated to
> > %SpiceUsbDevice.
> > > - *
> > > + * Returns: (transfer none): the %libusb_device associated to
> > %SpiceUsbDevice
> > > + * or NULL (if the device does not have associated libusb device)
> >
> > This is an exported function, and if we start returning NULL in some
> > cases, this is going to break applications using this API :(
> >
> >
> This means we'll need to send commit to gnome-boxes to check returned value.
> In general, when the external application (like gnome-boxes) uses spice-gtk
> and does not create devices that do not have libusb_device, it never
> find ones.
> Are there other uses of spice-gtk except of gnome-boxes?
If when you upgrade spice-gtk to a newer version, already installed apps
which are using spice-gtk start crashing, then I'd call this an ABI
break, which we want to avoid.. virt-viewer/remote-viewer is another
user, virt-manager too.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel