[Spice-devel] [PATCH v2 2/2] Avoid clang warnings on casts with stricter alignment requirements

[Christophe de Dinechin]

> On 23 May 2017, at 11:51, Christophe Fergeau <cfergeau at redhat.com> wrote:
> 
> On Thu, May 18, 2017 at 03:24:41PM +0200, Christophe de Dinechin wrote:
>>> 
>>>> @@ -140,6 +140,41 @@ size_t spice_strnlen(const char *str, size_t max_len);
>>>> 
>>>> #endif
>>>> 
>>>> +/* Cast to a type with stricter alignment constraints (to build with clang)
>>>> */
>>>> +extern void spice_alignment_warning(const char *loc, void *p, unsigned sz);
>>>> +extern void spice_alignment_debug(const char *loc, void *p, unsigned sz);
>>>> +
>>>> +static inline  void *spice_alignment_check(const char *loc,
>>>> +                                           void *ptr, unsigned sz)
>>>> +{
>>>> +#ifndef NDEBUG
>>> 
>>> I like the debug idea but I think that currently NDEBUG is never defined.
>> 
>> I read that package maintainers often build the released version by
>> doing ./configure CFLAGS=-DNDEBUG, and that it’s considered “principle
>> of least surprise” to use NDEBUG rather than something
>> package-specific for that purpose.
>> 
>> That being said, I checked our .spec file, and I don’t see NDEBUG
>> being set in there. That would be the logical next step.
> 
> I only see -DNDEBUG being defined for cmake builds in rpm --showrc, and
> none of my 250 spec files have a -DNDEBUG in them, so I would not expect
> this to be set.

Meaning none of the 250 spec files disable assert()… Or use it. Oh well ;-)


> I think we have SPICE_DEBUG already which might make
> sense

Actually, SPICE_DEBUG is a macro for writing a debug message. spice_assert() is not conditional (and spice_static_assert is broken).

> , another alternative would be to have a --enable-alignment-debug,

I tried that, but the problem is to keep that consistent between spice-common and spice-gdk (they may have different configuration). Do you have a similar case? If so, how do you deal with that?

(I could for example always leave the runtime routines in spice-common, even if checks are disabled there, so that you don’t get unsatisfied symbols. But that seems a bit ugly).


> or something like glib (SPICE_XXX_DEBUG=alignment:foo:bar)

Sorry, I did not really understand that. Do you mean a run-time flag check? If so, since Frediano was objecting a one-instruction runtime check, I think a runtime flag check would not convince him either. If you have a static configuration option in mind, could you give me an example of similar option?


Thanks
Christophe

> 
> Christophe
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel