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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx> 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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]