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

[Frediano Ziglio]

> 
> Hi,
> 
> On Tue, Aug 21, 2018 at 02:53:59PM +0200, Javier Celaya wrote:
> > > 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...
> 
> Either way is fine. If you open a bug, Frediano can attach his
> patch there later on.
> 
> I just think it is important to have the bug open and a comment
> in the code about it. I don't mind patching the code with the
> comment so we don't have to wait till Jorge is back ;)
> 
> Cheers,
> Victor
> 

In the meantime the macro is quite changed, it's now (master)

#if defined(g_has_typeof) && GLIB_VERSION_MAX_ALLOWED >= GLIB_VERSION_2_58
#define g_clear_pointer(pp, destroy)                                           \
  G_STMT_START {                                                               \
    G_STATIC_ASSERT (sizeof *(pp) == sizeof (gpointer));                       \
    __typeof__(*(pp)) _ptr = *(pp);                                            \
    *(pp) = NULL;                                                              \
    if (_ptr)                                                                  \
      (destroy) (_ptr);                                                        \
  } G_STMT_END
#else /* __GNUC__ */
#define g_clear_pointer(pp, destroy) \
  G_STMT_START {                                                               \
    G_STATIC_ASSERT (sizeof *(pp) == sizeof (gpointer));                       \
    /* Only one access, please; work around type aliasing */                   \
    union { char *in; gpointer *out; } _pp;                                    \
    gpointer _p;                                                               \
    /* This assignment is needed to avoid a gcc warning */                     \
    GDestroyNotify _destroy = (GDestroyNotify) (destroy);                      \
                                                                               \
    _pp.in = (char *) (pp);                                                    \
    _p = *_pp.out;                                                             \
    if (_p)                                                                    \
      {                                                                        \
        *_pp.out = NULL;                                                       \
        _destroy (_p);                                                         \
      }                                                                        \
  } G_STMT_END
#endif /* __GNUC__ */

so the first definition is good, calling directly destroy function,
on the second definition is ignoring the calling convention so basically
changing compiler you can have the bug or not.

The fix for the convention problem is to just call destroy without
converting it to GDestroyNotify (a void* should be fine in C although
in C++ won't work).

The 2 definitions are also incoherent, one is using pp once the other
twice so changing compilers you can have other side effects (note the
"Only one access, please" comment).

Frediano