[Spice-devel] [PATCH] Avoid passing libusb functions as callbacks

Tue Aug 21 12:53:59 UTC 2018

Hi,
El mar, 21-08-2018 a las 09:29 +0200, Victor Toso escribió:
> Hi,
> On Tue, Aug 21, 2018 at 03:18:35AM -0400, Frediano Ziglio wrote:
> 
> 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 at flexvdi.comwrot
> e:
> From: Jorge Olmos <jorge.olmos at flexvdi.com>
> When building spice-gtk for windows:- libusb uses __stdcall calling
> convention when compiled for win32.It doesnot include an option to be
> compiled with __cdecl callingconvention.Directly calling libusb
> functions works fine. But it is a problemwhen itsfunctions are passed
> as callbacks to a function that expects othercallingconvention.- glib
> uses __cdecl calling convention and expects the functions itreceives
> 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 becalledwith __cdecl convention. This
> causes stack corruption, and hencecrashes.
> Have you raised a bug in glib? We use this libraries to help
> withportability 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.
> Acompilation of glib can work well with __stdcall OR with __cdecl.
> Butnot with both of them.
> 
> It's possible, the function is called just once in the macro so
> won'tneed all the hackish casts that are currently present. I have a
> fixedversion of the macro (somewhere!).
> Ah, that would be cool.
> Right now I have just found another patch in spice-gtk, where the
> sameproblem was solved with the same solution, which also has
> noportability issues:
> https://patchwork.freedesktop.org/patch/92705/
> 
> Yes, this problem can cause a stack corruption (as you confirmed)
> andshould be merged GLib or not.
> Yes, not against merging the fix. But if we can get it fixed inGlib,
> it is matter to help other projects that port code towindows to be
> fixed as well, etc.
> Please open a bug in Glib and add a reference to it in the code

Are you talking to Jorge or to Frediano (since he has a fixed macro)?
Jorge is on holidays anyway, he will return in a couple of weeks. I can
open the bug report in Glib, but if you already have a working version
of the macro...
> (to avoid people reverting this change while the bug is not
> fixedthere)
>     https://gitlab.gnome.org/GNOME/glib/issues
> Cheers,Victor
> --- src/channel-usbredir.c | 9 ++++++--- 1 file changed, 6
> insertions(+), 3 deletions(-)
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.cindex
> 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_op
> en_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 @@
> voidspice_usbredir_channel_disconnect_device(SpiceUsbredirChannel*cha
> nnel)          /* This also closes the libusb handle we passed
> fromopen_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 listSpice-devel at lists.freedesktop.orghttps://lists.freedeskto
> p.org/mailman/listinfo/spice-devel
-- 


            Javier Celaya Alastrué
          
           Chief Technology Officer
      
      
        javier.celaya at flexvdi.com
        
        
        +34696969959 
      
          
        j_celaya
        
        
        Legal Information and Privacy Policy
          
      
            Política de confidencialidad
            Este mensaje y los ficheros anexos son confidenciales dirigiéndose exclusivamente al destinatario mencionado en el encabezamiento. Si usted ha recibido este correo por error, tenga la amabilidad de eliminarlo de su sistema y no divulgar el contenido a terceros. Los datos personales facilitados por usted o por terceros serán tratados por FLEXIBLE SOFTWARE SOLUTIONS S.L.U. con la finalidad de gestionar y mantener los contactos y relaciones que se produzcan como consecuencia de la relación que mantiene con FLEXIBLE SOFTWARE SOLUTIONS S.L.U. Normalmente, la base jurídica que legitima este tratamiento, será su consentimiento, el interés legítimo o la necesidad para gestionar una relación contractual o similar. El plazo de conservación de sus datos vendrá determinado por la relación que mantiene con nosotros. Para más información al respecto, o para ejercer sus derechos de acceso, rectificación, supresión, oposición, limitación o portabilidad, dirija una comunicación por escrito a FLEXIBLE SOFTWARE SOLUTIONS S.L.U: Avenida de Ranillas 1D, Planta 3, Oficina 3G, Zaragoza o al correo electrónico pdo at flexvdi.com. En caso de considerar vulnerado su derecho a la protección de datos personales, podrá interponer una reclamación ante la Agencia Española de Protección de Datos (www.agpd.es). 
        
    
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180821/23ff7f08/attachment-0001.html>