[Spice-devel] [PATCH spice-common 2/2] log: remove spice_static_assert macro

[Frediano Ziglio]

> 
> On Mon, Jun 12, 2017 at 07:46:07AM -0400, Marc-André Lureau wrote:
> > Hi
> > 
> > ----- Original Message -----
> > > On Mon, Jun 12, 2017 at 06:26:58AM -0400, Frediano Ziglio wrote:
> > > > > > 
> > > > > > > I think we should replace SPICE_VERIFY / verify with G_STATIC*
> > > > > > > equivalents.
> > > > > > >  
> > > > > > 
> > > > > > We actually can't for compatibility reason. Some version of GLib
> > > > > > have
> > > > > > broken macros and we still claim RHEL 6 support
> > > > > > 
> > > > > 
> > > > > Ah, I vaguely recall some discussion, do you have a pointer? I can't
> > > > > find
> > > > > changes in glib git log.
> > > > > 
> > > > 
> > > > I think Christophe F pointed out the problem. Don't remember exactly, I
> > > > think they were using array sizes but this do not work with C99.
> > > > Looking at current GLib master code is still using that broken
> > > > trick.
> > > 
> > > This is https://bugzilla.gnome.org/show_bug.cgi?id=758844
> > > Test case from the bug is
> > > 
> > > int main (int argc, char **argv)
> > > {
> > >     G_STATIC_ASSERT(argc == 4);
> > > 
> > >     return 0;
> > > }
> > > 
> > > which succeeds while the macro cannot know the value of argc.
> > > 
> > 
> > Thanks, too bad they didn't manage to introduce a verify-like solution.
> > 
> > (I wouldn't claim the glib macro to be broken though)
> 
> Misleading and potentially dangerous if you prefer ;)
> 
> int main (int argc, char **argv)
> {
>      G_STATIC_ASSERT(argc >= 4);
> 
>      /* All is well, argv is big enough */
> 
>      return argv[10];
> }
> 
> I don't think it's going to cause a lot of problems for real (even
> though I'd be tempted to use it as "hmm, not fully sure this value is
> known at compile-time, but G_STATIC_ASSERT will let me know anyway if
> what I'm attempting is bogus"), but since we have the choice between
> G_STATIC_ASSERT() and verify(), and verify() behaves better than
> G_STATIC_ASSERT(), we can as well use verify()
> 
> Christophe
> 

Back to the original patch. I don't see any side effects
about using SPICE_VERIFY instead of the old spice_static_assert
macro. And even if you plan to use another macro in the future
this patch is useful.

Frediano