Hi, On Fri, Jul 26, 2019 at 08:08:07AM +0300, Yuri Benditovich wrote: > On Thu, Jul 25, 2019 at 8:46 PM Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: > > > > > > @@ -1440,6 +1446,10 @@ gchar > > > > > > *spice_usb_device_get_description(SpiceUsbDevice > > > > > > *device, const gchar *for > > > > > > > > > > > > g_return_val_if_fail(device != NULL, NULL); > > > > > > > > > > > > + if (!device->bdev) { > > > > > > + return g_strdup(_("USB redirection")); > > > > > > + } > > > > > > + > > > > > > bus = spice_usb_device_get_busnum(device); > > > > > > address = spice_usb_device_get_devaddr(device); > > > > > > vid = spice_usb_device_get_vid(device); > > > > > > > > > Ok, now I had understand this patch. This is removing the > > assumption that bdev is never NULL. > > Only to support calling spice_usb_device_manager_device_error > > with a NULL device. > > I would say nack to this patch and find another solution. > > Maybe adding a "device_creation_error" signal with "error" > > but no device. > > IMO, creating special entity for each case that is little > different from existing ones is disrespect to Occam's principle > (and several similar ones). Heh, nice try. The difference here, IMO, would be that you have a clear objective: give an error when device creation fails. You want to do it by emit an error signal in a fake, empty device which is quite the workaround and I wouldn't call it a simpler alternative. > In context of 'device error signal' the 'device' is something > that can referenced/dereferenced and which name can be > retrieved. > > This is not a device error, it's a device manager error. > > We can view device manager as kind of device, then there is conflict. That upsets me a little. When I started learning the usb stack in spice-gtk to give some though on the design proposals, I saw lots of potential to the usb-backend work. What you proposes here goes in opposite direction of a clear definition of what each component of this does. So, I'd say also in reply to your previous argument around not defining an API. We can define an API an still change it before the next release, that's ok. It is also ok to deprecate it in the next release if we feel we did it wrong. But let's do it in the right way, trying to achieve something easy to understand and maintain. > > > This is caused by wanting to use an interface (properties) > > that does not allow to return an error instead. > > As any solution, this one has pros and cons. From my personal > point of view, it has significant pro (low cost of > implementation) and does not have any significant con. Cheers, Victor
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel