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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel