Hi Frediano, Thanks for your feedback, that's exactly what I was looking for ahead of opening a MR. On Thu, Dec 23, 2021 at 4:21 AM Frediano Ziglio <freddy77@xxxxxxxxx> wrote: > could you open a MR? Is this the right copy of the repo to make an MR against? https://gitlab.freedesktop.org/spice/spice-gtk > I suppose you are speaking about allocate_device_for_file_descriptor. It's not called anywhere in the code so I suppose it is a public API. In this case it should start with something like spice_. Also it should have a bit more documentation and be listed in src/spice-glib-sym-file. But src/usb-backend.h is not a public header and we don't want it to be so it does not seem a good idea to declare there. That makes sense. However, if in a new implementation it would make sense for UsbDeviceManager to use some functionality from UsbBackend, I'd still need to declare it in usb-backend.h, right? Just as an aside, is there a way to make a method available to UsbDeviceManager but unavailable publicly? > This seems like a bad idea. It exposes something internal. Why not having just one additional function in device manager like > > gboolean > spice_usb_device_manager_add_device_from_fd(SpiceUsbDeviceManager *manager, GError **err); This is a good suggestion. You're essentially proposing for UsbDeviceManager (which is public) to be the one exposing the functionality and for it to potentially use some functionality left in UsbBackend (which is private) internally. I'll refactor. > The device could be communicated using device_added signal (if results if not FALSE), error can be returned, no additional interface to expose. > Or returning directory a SpiceUsbDevice* instead of a gboolean (but in this case interface will have to be blocking). Any pros / cons with either one? Any preference? If I return SpiceUsbDevice * directly, there is no need for the err, parameter and I could just return NULL device if something went wrong. Is there any preference or rule of thumb that the project generally follows? Thanks! iordan