> > Hi, > On Fri, 2018-08-17 at 15:24 +0200, Victor Toso wrote: > > Hi, > > > > On Fri, Aug 17, 2018 at 03:12:35PM +0200, jorge.olmos@xxxxxxxxxxx > > wrote: > > > From: Jorge Olmos <jorge.olmos@xxxxxxxxxxx> > > > > > > When building spice-gtk for windows: > > > - libusb uses __stdcall calling convention when compiled for win32. > > > It does > > > not include an option to be compiled with __cdecl calling > > > convention. > > > Directly calling libusb functions works fine. But it is a problem > > > when its > > > functions are passed as callbacks to a function that expects other > > > calling > > > convention. > > > - glib uses __cdecl calling convention and expects the functions it > > > receives as parameters to follow __cdecl convention. > > > > > > So the lines included in spice-gtk like: > > > g_clear_pointer(&priv->device, libusb_unref_device); > > > cause libusb_unref_device (compiled with _stdcall convention) to be > > > called > > > with __cdecl convention. This causes stack corruption, and hence > > > crashes. > > > > Have you raised a bug in glib? We use this libraries to help with > > portability so I'd hope it is possible to fix in glib. > > > > I don't think it is even possible to fix this cleanly in glib. A > compilation of glib can work well with __stdcall OR with __cdecl. But > not with both of them. > It's possible, the function is called just once in the macro so won't need all the hackish casts that are currently present. I have a fixed version of the macro (somewhere!). > Right now I have just found another patch in spice-gtk, where the same > problem was solved with the same solution, which also has no > portability issues: > > https://patchwork.freedesktop.org/patch/92705/ > Yes, this problem can cause a stack corruption (as you confirmed) and should be merged GLib or not. > > > > > --- > > > src/channel-usbredir.c | 9 ++++++--- > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c > > > index 6ffe546..1d9c380 100644 > > > --- a/src/channel-usbredir.c > > > +++ b/src/channel-usbredir.c > > > @@ -352,7 +352,8 @@ static void spice_usbredir_channel_open_acl_cb( > > > spice_usbredir_channel_open_device(channel, &err); > > > } > > > if (err) { > > > - g_clear_pointer(&priv->device, libusb_unref_device); > > > + libusb_unref_device(priv->device); > > > + priv->device = NULL; > > > g_boxed_free(spice_usb_device_get_type(), priv- > > > >spice_device); > > > priv->spice_device = NULL; > > > priv->state = STATE_DISCONNECTED; > > > @@ -383,7 +384,8 @@ _open_device_async_cb(GTask *task, > > > spice_usbredir_channel_lock(channel); > > > > > > if (!spice_usbredir_channel_open_device(channel, &err)) { > > > - g_clear_pointer(&priv->device, libusb_unref_device); > > > + libusb_unref_device(priv->device); > > > + priv->device = NULL; > > > g_boxed_free(spice_usb_device_get_type(), priv- > > > >spice_device); > > > priv->spice_device = NULL; > > > } > > > @@ -504,7 +506,8 @@ void > > > spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel > > > *channel) > > > > > > /* This also closes the libusb handle we passed from > > > open_device */ > > > usbredirhost_set_device(priv->host, NULL); > > > - g_clear_pointer(&priv->device, libusb_unref_device); > > > + libusb_unref_device(priv->device); > > > + priv->device = NULL; > > > g_boxed_free(spice_usb_device_get_type(), priv- > > > >spice_device); > > > priv->spice_device = NULL; > > > priv->state = STATE_DISCONNECTED; Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel