[Spice-devel] [spice-gtk 08/13] usb-redir: change signal prototype of win-usb-dev

Christophe Fergeau cfergeau at redhat.com
Thu Mar 14 08:36:25 UTC 2019


On Wed, Mar 13, 2019 at 08:14:14PM +0200, Yuri Benditovich wrote:
> On Tue, Mar 12, 2019 at 4:02 PM Christophe Fergeau <cfergeau at redhat.com> wrote:
> >
> > On Sun, Mar 10, 2019 at 04:46:07PM +0200, Yuri Benditovich wrote:
> > > Changing signal definition from (boxed-boxed) to (pointer,int).
> > > There is no need for additional referencing of GUdevDevice
> > > object before signal callback.
> >
> > I still feel it would be nicer to guarantee the GUdevDevice will stay
> > alive for the whole duration of the signal emission. For example, 2
> > callbacks may be attached to this signal, we don't want the first one to
> > be able to drop the last ref to the GUdevDevice and risking the second
> > one trying to use freed memory.
> >
> Additional referencing here is not required. The signal callback does not
> dereference the GUdevDevice object, so this does not make any difference
> how many callbacks attached to the signal.

Yes, right now it does not make any difference. But maybe that will make
a difference for someone hacking on that code in a few years. And
gobject signals usually own a reference on signals parameters while the
callbacks are running. Hence the suggestion ;)

> 
> 
> 
> > > Second parameter (action) is 0 for device removal and 1 for device
> > > addition.
> >
> > Imo this should either be a gboolean or an enum.
> 
> I do not see any reason to invent additional marshalling procedure where
> existing one can be used without any problem.

Code readability. More or less the same reasons why we have
gboolean/bool while int does the job. Someone looking at the callback
won't wonder why only 0 and 1 are handled, and won't be worried whether
they can get other values in the callback.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190314/c92e759d/attachment.sig>


More information about the Spice-devel mailing list