Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Hi,
>>>
>>> I found some code which was referencing directly some
>>> CONFIG_XENO_OPT_DEBUG_ variables with things like:
>>>
>>> #ifdef CONFIG_XENO_OPT_DEBUG_FOO
>>>
>>> This usage is incompatible with the pre-requisites of the assert.h
>>> header that CONFIG_XENO_OPT_DEBUG_FOO should be defined at all times.
>>> While grepping for CONFIG_XENO_OPT_DEBUG_, I found that we also have
>>> many duplicates of construction like:
>>> #ifndef CONFIG_XENO_OPT_DEBUG_FOO
>>> #define CONFIG_XENO_OPT_DEBUG_FOO 0
>>> #endif /* CONFIG_XENO_OPT_DEBUG_FOO */
>>>
>>> So, a patch follows which:
>>> - replace the #ifdef with some #if XENO_DEBUG(FOO)
>> Should probably come as a separate patch.
> 
> Come on, the patch is simple, one patch for this is enough.

One part is an obvious fix, the other a refactoring under discussion.

> 
>>> - move all the initializations to assert.h
>>>
>>> This will make any reference to CONFIG_XENO_OPT_DEBUG_FOO outside of
>>> assert.h suspicious, and easy to detect.
>> How many duplicates did you find?
> 
> A lot, especially for CONFIG_XENO_OPT_DEBUG_NUCLEUS

That's because the CONFIG_XENO_OPT_DEBUG_NUCLEUS defines were misplaced.
Besides that we had one CONFIG_XENO_OPT_DEBUG_QUEUE duplicate, but tons
of proper users. I don't see such an immediate need.

When adding this kind of switching, it's till more handy to have things
locally in your subsystem. That also makes reviewing easier when you
only find changes in files that belong to a subsystem.

> 
>> Generally, I'm more a fan of decentralized management here (e.g. this
>> avoids needless patch conflicts in central files).
> 
> If we maintain the list in alphabetical order (which I have done), we 
> reduce the likeliness for conflicts. The aim of doing this is also that 
> I can check that the sources are clean with:
> 
> find xenomai-2.5 ! -name 'assert.h' -name '*.[ch]' | xargs grep 
> CONFIG_XENO_OPT_DEBUG_
> 
> And that I can add this test to the automated build test.
> 
> Note that forgetting to add a #define to the list yields an immediate
> compilation error. So, the patch makes things completely safe.

What did you change to make this happen (for new users of XENO_DEBUG)?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core

Reply via email to