[Spice-devel] [spice-server] build-sys: Improve ENABLE_EXTRA_CHECK setting

[Christophe Fergeau]

On Tue, Sep 12, 2017 at 07:28:59AM -0400, Frediano Ziglio wrote:
> > 
> > On Tue, Sep 12, 2017 at 06:55:41AM -0400, Frediano Ziglio wrote:
> > > > 
> > > > On Tue, Sep 12, 2017 at 06:42:16AM -0400, Frediano Ziglio wrote:
> > > > > In this case you are introducing a regression, current code is
> > > > > designed to support this already. I cannot surely say that this
> > > > > patch is an improvement.
> > > > 
> > > > By that reasoning, any patch which makes some functions static or remove
> > > > some unused functions is a regression.
> > > > 
> > > > Christophe
> > > > 
> > > 
> > > Depends on the design. In this case is designed to be reused.
> > 
> > If you want
> > 1) that the same ENABLE_EXTRA_CHECKS symbol name is reused in my v1
> > 2) that it's globally #defined to 0/1
> > then yes, we are back to square 1, we should never use #ifndef/#ifdef with
> > it,
> > which my suggestion and then patch was trying to address.
> > 
> > Christophe
> > 
> 
> Let me recap, there are 3 different topic:
> 
> 1- defined/not defined. We agree that the "standard" for config.h
>    is defined to 1 or undefined, in this respect the patch to
>    configure.ac goes into the right direction;
> 2- using same name. Opinion but looks like we don't disagree;
> 3- make the variable available globally (maybe in red-common.h?).
>    Yes, this is not documented as a global feature to reuse.
>    I won't personally have 2 patches for this change.
>    I personally think that having in a global header well documented
>    why this is here would be the right solution. We added just couple
>    of weeks ago with this intention, just badly documented and
>    prone to get removed.

All I'm saying is that it seems complicated to have the 3 things at the
same time :) My patches were going with 1) and 2) because I was not
aware of the requirement for 3). If 3) is a must, then I'm not sure we
can have both 1) and 2).

Christophe