Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
On Mon, 2010-04-19 at 15:58 +0200, 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) - move all the initializations to assert.h Yes, that makes a lot of sense. Declaring DEBUG options locally was a sloppy fix for this annoying issue I used a lot myself. This has to be centralized somewhere. This will make any reference to CONFIG_XENO_OPT_DEBUG_FOO outside of assert.h suspicious, and easy to detect. Thanks in advance for any comments. Regards. -- Philippe. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Jan Kiszka wrote: 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. That is not the main point, the main point is that putting all these defines in one files allow to detect easily direct references to the symbols. 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)? Nothing. If you forget to add the define, and do not enable the debug option (which everybody does most of the time), you get a compilation error. What I changed, however, is that the above find | grep allows immediately to detect direct users of the symbols. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: 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. That is not the main point, the main point is that putting all these defines in one files allow to detect easily direct references to the symbols. 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)? Nothing. If you forget to add the define, and do not enable the debug option (which everybody does most of the time), you get a compilation error. The alternative (and decentralized) approach for fixing this consists of Kconfig magic for generating the value: config XENO_OPT_DEBUG_FOO bool ... config XENO_OPT_DEBUG_FOO_P int default 1 if XENO_OPT_DEBUG_FOO default 0 and XENO_DEBUG() could be extended to test for CONFIG_XENO_OPT_DEBUG_FOO_P when given FOO. I'm just not sure if this can be expressed for legacy 2.4 kernels, so it might have to wait for Xenomai 3. We can do something like that. But I still find this more complicated than simply moving three lines to assert.h. What is your problem exactly? Do you have some customer code which defines some more debug symbols? In that case there is no problem, you can keep the code as is. I plan to do the debug check part of the build test, not part of the makefiles. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Gilles Chanteperdrix wrote: Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: 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. That is not the main point, the main point is that putting all these defines in one files allow to detect easily direct references to the symbols. 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)? Nothing. If you forget to add the define, and do not enable the debug option (which everybody does most of the time), you get a compilation error. The alternative (and decentralized) approach for fixing this consists of Kconfig magic for generating the value: config XENO_OPT_DEBUG_FOO bool ... config XENO_OPT_DEBUG_FOO_P int default 1 if XENO_OPT_DEBUG_FOO default 0 and XENO_DEBUG() could be extended to test for CONFIG_XENO_OPT_DEBUG_FOO_P when given FOO. I'm just not sure if this can be expressed for legacy 2.4 kernels, so it might have to wait for Xenomai 3. We can do something like that. But I still find this more complicated than simply moving three lines to assert.h. It requires one more line but provides the safety the current approach lacks - not worth it? What is your problem exactly? Do you have some customer code which defines some more debug symbols? In that case there is no problem, you can keep the code as is. I plan to do the debug check part of the build test, not part of the makefiles. I have no such problems or concerns. I just find the centralization an unclean workaround for the actual issue. 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
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
On Mon, 2010-04-19 at 17:58 +0200, Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: 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. That is not the main point, the main point is that putting all these defines in one files allow to detect easily direct references to the symbols. 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)? Nothing. If you forget to add the define, and do not enable the debug option (which everybody does most of the time), you get a compilation error. The alternative (and decentralized) approach for fixing this consists of Kconfig magic for generating the value: config XENO_OPT_DEBUG_FOO bool ... config XENO_OPT_DEBUG_FOO_P int default 1 if XENO_OPT_DEBUG_FOO default 0 and XENO_DEBUG() could be extended to test for CONFIG_XENO_OPT_DEBUG_FOO_P when given FOO. I'm just not sure if this can be expressed for legacy 2.4 kernels, so it might have to wait for Xenomai 3. Well, actually, I would not merge this in Xenomai 3. I find this rather overkill; mainline first I mean, and mainline, i.e. the Xenomai code base only requires a simple and straightforward way to get debug switches right. Having to make Kconfig a kitchen sink for some unknown out of tree modules to be happy is not really my preferred approach in this particular case. Don't get me wrong, I'm not opposed to a more decentralized approach on the paper, it's just that I only care about the mainline tree here. We can do something like that. But I still find this more complicated than simply moving three lines to assert.h. It requires one more line but provides the safety the current approach lacks - not worth it? What is your problem exactly? Do you have some customer code which defines some more debug symbols? In that case there is no problem, you can keep the code as is. I plan to do the debug check part of the build test, not part of the makefiles. I have no such problems or concerns. I just find the centralization an unclean workaround for the actual issue. 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 -- Philippe. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Jan Kiszka wrote:
Philippe Gerum wrote:
config XENO_OPT_DEBUG_FOO
bool ...
config XENO_OPT_DEBUG_FOO_P
int
default 1 if XENO_OPT_DEBUG_FOO
default 0
and XENO_DEBUG() could be extended to test for
CONFIG_XENO_OPT_DEBUG_FOO_P when given FOO. I'm just not sure if this
can be expressed for legacy 2.4 kernels, so it might have to wait for
Xenomai 3.
Well, actually, I would not merge this in Xenomai 3. I find this rather
overkill; mainline first I mean, and mainline, i.e. the Xenomai code
base only requires a simple and straightforward way to get debug
switches right. Having to make Kconfig a kitchen sink for some unknown
out of tree modules to be happy is not really my preferred approach in
this particular case.
Don't get me wrong, I'm not opposed to a more decentralized approach on
the paper, it's just that I only care about the mainline tree here.
The point is not out-of-tree but robustness. Neither the current
decentralized #ifdef-#define nor its centralized brother meet this
criteria. An approach like the above which forces you to provide all
required bits before any of the cases (disabled/enabled) starts to work
does so.
Ok. What about:
#define __name2(a, b) a ## b
#define name2(a, b) __name2(a, b)
#define DECLARE_ASSERT_SYMBOL(sym) \
static const int CONFIG_XENO_OPT_DEBUG_##sym##0 = 0,\
__CONFIG_XENO_OPT_DEBUG_##sym =
name2(CONFIG_XENO_OPT_DEBUG_##sym, 0)
#define XENO_ASSERT(subsystem,cond,action) do { \
if (unlikely(__CONFIG_XENO_OPT_DEBUG_##subsystem 0 !(cond))) { \
xnarch_trace_panic_freeze(); \
xnlogerr(assertion failed at %s:%d (%s)\n, __FILE__, __LINE__,
(#cond)); \
xnarch_trace_panic_dump(); \
action; \
} \
} while(0)
DECLARE_ASSERT_SYMBOL(NUCLEUS);
It fails to compile when the debug symbol is set and
DECLARE_ASSERT_SYMBOL is missing, which plugs the failure of my previous
attempt.
--
Gilles.
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Philippe Gerum wrote: On Mon, 2010-04-19 at 18:25 +0200, Philippe Gerum wrote: On Mon, 2010-04-19 at 18:14 +0200, Jan Kiszka wrote: Philippe Gerum wrote: On Mon, 2010-04-19 at 17:58 +0200, Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: 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. That is not the main point, the main point is that putting all these defines in one files allow to detect easily direct references to the symbols. 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)? Nothing. If you forget to add the define, and do not enable the debug option (which everybody does most of the time), you get a compilation error. The alternative (and decentralized) approach for fixing this consists of Kconfig magic for generating the value: config XENO_OPT_DEBUG_FOO bool ... config XENO_OPT_DEBUG_FOO_P int default 1 if XENO_OPT_DEBUG_FOO default 0 and XENO_DEBUG() could be extended to test for CONFIG_XENO_OPT_DEBUG_FOO_P when given FOO. I'm just not sure if this can be expressed for legacy 2.4 kernels, so it might have to wait for Xenomai 3. Well, actually, I would not merge this in Xenomai 3. I find this rather overkill; mainline first I mean, and mainline, i.e. the Xenomai code base only requires a simple and straightforward way to get debug switches right. Having to make Kconfig a kitchen sink for some unknown out of tree modules to be happy is not really my preferred approach in this particular case. Don't get me wrong, I'm not opposed to a more decentralized approach on the paper, it's just that I only care about the mainline tree here. The point is not out-of-tree but robustness. Neither the current decentralized #ifdef-#define nor its centralized brother meet this criteria. An approach like the above which forces you to provide all required bits before any of the cases (disabled/enabled) starts to work does so. Flexibility and robustness have to be combined. Explicit declaration in Kconfig is against flexibility, because this is one external thing more to take care of, for adding something as simple as a debug switch. So if You already have to take care of Kconfig if you want a debug switch. 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
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
On Mon, 2010-04-19 at 19:22 +0200, Jan Kiszka wrote: Philippe Gerum wrote: On Mon, 2010-04-19 at 18:25 +0200, Philippe Gerum wrote: On Mon, 2010-04-19 at 18:14 +0200, Jan Kiszka wrote: Philippe Gerum wrote: On Mon, 2010-04-19 at 17:58 +0200, Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: 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. That is not the main point, the main point is that putting all these defines in one files allow to detect easily direct references to the symbols. 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)? Nothing. If you forget to add the define, and do not enable the debug option (which everybody does most of the time), you get a compilation error. The alternative (and decentralized) approach for fixing this consists of Kconfig magic for generating the value: config XENO_OPT_DEBUG_FOO bool ... config XENO_OPT_DEBUG_FOO_P int default 1 if XENO_OPT_DEBUG_FOO default 0 and XENO_DEBUG() could be extended to test for CONFIG_XENO_OPT_DEBUG_FOO_P when given FOO. I'm just not sure if this can be expressed for legacy 2.4 kernels, so it might have to wait for Xenomai 3. Well, actually, I would not merge this in Xenomai 3. I find this rather overkill; mainline first I mean, and mainline, i.e. the Xenomai code base only requires a simple and straightforward way to get debug switches right. Having to make Kconfig a kitchen sink for some unknown out of tree modules to be happy is not really my preferred approach in this particular case. Don't get me wrong, I'm not opposed to a more decentralized approach on the paper, it's just that I only care about the mainline tree here. The point is not out-of-tree but robustness. Neither the current decentralized #ifdef-#define nor its centralized brother meet this criteria. An approach like the above which forces you to provide all required bits before any of the cases (disabled/enabled) starts to work does so. Flexibility and robustness have to be combined. Explicit declaration in Kconfig is against flexibility, because this is one external thing more to take care of, for adding something as simple as a debug switch. So if You already have to take care of Kconfig if you want a debug switch. Sure, but I don't find useful to clutter it even more to get it working. Jan -- Philippe. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Jan Kiszka wrote:
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
Philippe Gerum wrote:
config XENO_OPT_DEBUG_FOO
bool ...
config XENO_OPT_DEBUG_FOO_P
int
default 1 if XENO_OPT_DEBUG_FOO
default 0
and XENO_DEBUG() could be extended to test for
CONFIG_XENO_OPT_DEBUG_FOO_P when given FOO. I'm just not sure if
this
can be expressed for legacy 2.4 kernels, so it might have to wait for
Xenomai 3.
Well, actually, I would not merge this in Xenomai 3. I find this rather
overkill; mainline first I mean, and mainline, i.e. the Xenomai code
base only requires a simple and straightforward way to get debug
switches right. Having to make Kconfig a kitchen sink for some unknown
out of tree modules to be happy is not really my preferred approach in
this particular case.
Don't get me wrong, I'm not opposed to a more decentralized approach on
the paper, it's just that I only care about the mainline tree here.
The point is not out-of-tree but robustness. Neither the current
decentralized #ifdef-#define nor its centralized brother meet this
criteria. An approach like the above which forces you to provide all
required bits before any of the cases (disabled/enabled) starts to work
does so.
Ok. What about:
#define __name2(a, b) a ## b
#define name2(a, b) __name2(a, b)
#define DECLARE_ASSERT_SYMBOL(sym) \
static const int CONFIG_XENO_OPT_DEBUG_##sym##0 = 0,\
__CONFIG_XENO_OPT_DEBUG_##sym =
name2(CONFIG_XENO_OPT_DEBUG_##sym, 0)
#define XENO_ASSERT(subsystem,cond,action) do { \
if (unlikely(__CONFIG_XENO_OPT_DEBUG_##subsystem 0 !(cond))) { \
xnarch_trace_panic_freeze(); \
xnlogerr(assertion failed at %s:%d (%s)\n, __FILE__, __LINE__,
(#cond)); \
xnarch_trace_panic_dump(); \
action; \
} \
} while(0)
DECLARE_ASSERT_SYMBOL(NUCLEUS);
It fails to compile when the debug symbol is set and
DECLARE_ASSERT_SYMBOL is missing, which plugs the failure of my previous
attempt.
I'm still wrapping my head around this. What would be the usage,
#ifndef CONFIG_XENO_OPT_DEBUG_FOO
#define CONFIG_XENO_OPT_DEBUG_FOO 0
#endif
DECLARE_ASSERT_SYMBOL(FOO);
? If the compiler is smart enough to still drop the asserts based on
static const, I'm fine as this is an improvement.
No, you just use DECLARE_ASSERT_SYMBOL(FOO)
Would be nice - if it worked.
Still, IMHO, this solution would not even win the second league beauty
contest (now it comes with as many additional lines as the
Kconfig-approach).
Yes, it is not pretty but to add a config option you just add the usual
Kconfig stuff, then DECLARE_ASSERT_SYMBOL in the code instead of the
#ifndef #define foo 0 #endif.
If you do not do it, you get a compilation error whether the option is
enabled or not.
It can be decentralized, the find | grep mentioned earlier will still work.
If we can make it work like that, I'm all for it. But:
error: initializer element is not constant
(when disabled)
Right, I get this one. I only tested with the preprocessor.
or
error: ‘y0’ undeclared here (not in a function)
(when enabled)
However, it works for me when enabled. Trying harder now.
--
Gilles.
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Jan Kiszka wrote:
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
Philippe Gerum wrote:
config XENO_OPT_DEBUG_FOO
bool ...
config XENO_OPT_DEBUG_FOO_P
int
default 1 if XENO_OPT_DEBUG_FOO
default 0
and XENO_DEBUG() could be extended to test for
CONFIG_XENO_OPT_DEBUG_FOO_P when given FOO. I'm just not sure if
this
can be expressed for legacy 2.4 kernels, so it might have to wait for
Xenomai 3.
Well, actually, I would not merge this in Xenomai 3. I find this rather
overkill; mainline first I mean, and mainline, i.e. the Xenomai code
base only requires a simple and straightforward way to get debug
switches right. Having to make Kconfig a kitchen sink for some unknown
out of tree modules to be happy is not really my preferred approach in
this particular case.
Don't get me wrong, I'm not opposed to a more decentralized approach on
the paper, it's just that I only care about the mainline tree here.
The point is not out-of-tree but robustness. Neither the current
decentralized #ifdef-#define nor its centralized brother meet this
criteria. An approach like the above which forces you to provide all
required bits before any of the cases (disabled/enabled) starts to work
does so.
Ok. What about:
#define __name2(a, b) a ## b
#define name2(a, b) __name2(a, b)
#define DECLARE_ASSERT_SYMBOL(sym) \
static const int CONFIG_XENO_OPT_DEBUG_##sym##0 = 0,\
__CONFIG_XENO_OPT_DEBUG_##sym =
name2(CONFIG_XENO_OPT_DEBUG_##sym, 0)
#define XENO_ASSERT(subsystem,cond,action) do { \
if (unlikely(__CONFIG_XENO_OPT_DEBUG_##subsystem 0 !(cond))) { \
xnarch_trace_panic_freeze(); \
xnlogerr(assertion failed at %s:%d (%s)\n, __FILE__, __LINE__,
(#cond)); \
xnarch_trace_panic_dump(); \
action; \
} \
} while(0)
DECLARE_ASSERT_SYMBOL(NUCLEUS);
It fails to compile when the debug symbol is set and
DECLARE_ASSERT_SYMBOL is missing, which plugs the failure of my previous
attempt.
I'm still wrapping my head around this. What would be the usage,
#ifndef CONFIG_XENO_OPT_DEBUG_FOO
#define CONFIG_XENO_OPT_DEBUG_FOO 0
#endif
DECLARE_ASSERT_SYMBOL(FOO);
? If the compiler is smart enough to still drop the asserts based on
static const, I'm fine as this is an improvement.
No, you just use DECLARE_ASSERT_SYMBOL(FOO)
Would be nice - if it worked.
Still, IMHO, this solution would not even win the second league beauty
contest (now it comes with as many additional lines as the
Kconfig-approach).
Yes, it is not pretty but to add a config option you just add the usual
Kconfig stuff, then DECLARE_ASSERT_SYMBOL in the code instead of the
#ifndef #define foo 0 #endif.
If you do not do it, you get a compilation error whether the option is
enabled or not.
It can be decentralized, the find | grep mentioned earlier will still work.
If we can make it work like that, I'm all for it. But:
error: initializer element is not constant
(when disabled)
or
error: ‘y0’ undeclared here (not in a function)
(when enabled)
I'm afraid the preprocessor is not powerful enough for this task (we
would need macros that include preprocessor conditionals).
The following seems to work for me:
#define __name2(a, b) a ## b
#define name2(a, b) __name2(a, b)
#define DECLARE_ASSERT_SYMBOL(sym) \
static const int CONFIG_XENO_OPT_DEBUG_##sym##0 = 0
#define XENO_DEBUG(sym) (name2(CONFIG_XENO_OPT_DEBUG_##sym,0) 0)
#define XENO_ASSERT(subsystem,cond,action) do { \
if (unlikely(XENO_DEBUG(subsystem) !(cond))) { \
xnarch_trace_panic_freeze(); \
xnlogerr(assertion failed at %s:%d (%s)\n, __FILE__, __LINE__,
(#cond)); \
xnarch_trace_panic_dump(); \
action; \
} \
} while(0)
DECLARE_ASSERT_SYMBOL(NUCLEUS);
Jan
--
Gilles.
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
Philippe Gerum wrote:
config XENO_OPT_DEBUG_FOO
bool ...
config XENO_OPT_DEBUG_FOO_P
int
default 1 if XENO_OPT_DEBUG_FOO
default 0
and XENO_DEBUG() could be extended to test for
CONFIG_XENO_OPT_DEBUG_FOO_P when given FOO. I'm just not sure if
this
can be expressed for legacy 2.4 kernels, so it might have to wait for
Xenomai 3.
Well, actually, I would not merge this in Xenomai 3. I find this rather
overkill; mainline first I mean, and mainline, i.e. the Xenomai code
base only requires a simple and straightforward way to get debug
switches right. Having to make Kconfig a kitchen sink for some unknown
out of tree modules to be happy is not really my preferred approach in
this particular case.
Don't get me wrong, I'm not opposed to a more decentralized approach on
the paper, it's just that I only care about the mainline tree here.
The point is not out-of-tree but robustness. Neither the current
decentralized #ifdef-#define nor its centralized brother meet this
criteria. An approach like the above which forces you to provide all
required bits before any of the cases (disabled/enabled) starts to work
does so.
Ok. What about:
#define __name2(a, b) a ## b
#define name2(a, b) __name2(a, b)
#define DECLARE_ASSERT_SYMBOL(sym)
\
static const int CONFIG_XENO_OPT_DEBUG_##sym##0 = 0,\
__CONFIG_XENO_OPT_DEBUG_##sym =
name2(CONFIG_XENO_OPT_DEBUG_##sym, 0)
#define XENO_ASSERT(subsystem,cond,action) do { \
if (unlikely(__CONFIG_XENO_OPT_DEBUG_##subsystem 0 !(cond))) { \
xnarch_trace_panic_freeze(); \
xnlogerr(assertion failed at %s:%d (%s)\n, __FILE__, __LINE__,
(#cond)); \
xnarch_trace_panic_dump(); \
action; \
} \
} while(0)
DECLARE_ASSERT_SYMBOL(NUCLEUS);
It fails to compile when the debug symbol is set and
DECLARE_ASSERT_SYMBOL is missing, which plugs the failure of my previous
attempt.
I'm still wrapping my head around this. What would be the usage,
#ifndef CONFIG_XENO_OPT_DEBUG_FOO
#define CONFIG_XENO_OPT_DEBUG_FOO 0
#endif
DECLARE_ASSERT_SYMBOL(FOO);
? If the compiler is smart enough to still drop the asserts based on
static const, I'm fine as this is an improvement.
No, you just use DECLARE_ASSERT_SYMBOL(FOO)
Would be nice - if it worked.
Still, IMHO, this solution would not even win the second league beauty
contest (now it comes with as many additional lines as the
Kconfig-approach).
Yes, it is not pretty but to add a config option you just add the usual
Kconfig stuff, then DECLARE_ASSERT_SYMBOL in the code instead of the
#ifndef #define foo 0 #endif.
If you do not do it, you get a compilation error whether the option is
enabled or not.
It can be decentralized, the find | grep mentioned earlier will still work.
If we can make it work like that, I'm all for it. But:
error: initializer element is not constant
(when disabled)
or
error: ‘y0’ undeclared here (not in a function)
(when enabled)
I'm afraid the preprocessor is not powerful enough for this task (we
would need macros that include preprocessor conditionals).
The following seems to work for me:
#define __name2(a, b) a ## b
#define name2(a, b) __name2(a, b)
#define DECLARE_ASSERT_SYMBOL(sym) \
static const int CONFIG_XENO_OPT_DEBUG_##sym##0 = 0
#define XENO_DEBUG(sym) (name2(CONFIG_XENO_OPT_DEBUG_##sym,0) 0)
#define XENO_ASSERT(subsystem,cond,action) do { \
if (unlikely(XENO_DEBUG(subsystem) !(cond))) { \
xnarch_trace_panic_freeze(); \
xnlogerr(assertion failed at %s:%d (%s)\n, __FILE__, __LINE__,
(#cond)); \
xnarch_trace_panic_dump(); \
action; \
} \
} while(0)
DECLARE_ASSERT_SYMBOL(NUCLEUS);
We only loose the detection of the debug symbol used and not declared if
it is enabled. But this looks to me like a minor issue. Still trying though.
--
Gilles.
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
My compiler still complains about undefined 'y0' in the enabled case.
I'll try to dig into a different direction now: Automatic generation
during build. This is what the kernel does as well when the preprocessor
gives up. Would even save the DECLARE and should make everyone happy.
No, please nothing like that.
Because ... ?
BTW, I'll extend the prepare stage. Defining the proper dependencies for
build-time generation gets too hairy.
This one works for me:
#include stdlib.h
#include stdio.h
#define __name2(a, b) a ## b
#define name2(a, b) __name2(a, b)
#define DECLARE_ASSERT_SYMBOL(sym) \
static const int XENO_OPT_DEBUG_##sym = 0; \
static const int CONFIG_XENO_OPT_DEBUG_##sym##0 = 0
#define XENO_DEBUG(sym) \
(name2(CONFIG_XENO_OPT_DEBUG_##sym,0) XENO_OPT_DEBUG_##sym)
#define XENO_ASSERT(subsystem,cond,action) do { \
if (unlikely(XENO_DEBUG(subsystem) !(cond))) { \
xnarch_trace_panic_freeze(); \
xnlogerr(assertion failed at %s:%d (%s)\n, __FILE__, __LINE__,
(#cond)); \
xnarch_trace_panic_dump(); \
action; \
} \
} while(0)
DECLARE_ASSERT_SYMBOL(NUCLEUS);
int main(void)
{
if (XENO_DEBUG(NUCLEUS))
printf(Hello\n);
}
Please try and send me the result of pre-processing if
it does not work for you.
Find it attached.
Jan
# 1 test-debug1.c
# 1 built-in
# 1 command-line
# 1 test-debug1.c
# 1 /usr/include/stdlib.h 1 3 4
# 25 /usr/include/stdlib.h 3 4
# 1 /usr/include/features.h 1 3 4
# 330 /usr/include/features.h 3 4
# 1 /usr/include/sys/cdefs.h 1 3 4
# 348 /usr/include/sys/cdefs.h 3 4
# 1 /usr/include/bits/wordsize.h 1 3 4
# 349 /usr/include/sys/cdefs.h 2 3 4
# 331 /usr/include/features.h 2 3 4
# 354 /usr/include/features.h 3 4
# 1 /usr/include/gnu/stubs.h 1 3 4
# 1 /usr/include/bits/wordsize.h 1 3 4
# 5 /usr/include/gnu/stubs.h 2 3 4
# 1 /usr/include/gnu/stubs-64.h 1 3 4
# 10 /usr/include/gnu/stubs.h 2 3 4
# 355 /usr/include/features.h 2 3 4
# 26 /usr/include/stdlib.h 2 3 4
# 1 /usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h 1 3 4
# 214 /usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h 3 4
typedef long unsigned int size_t;
# 326 /usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h 3 4
typedef int wchar_t;
# 34 /usr/include/stdlib.h 2 3 4
# 96 /usr/include/stdlib.h 3 4
typedef struct
{
int quot;
int rem;
} div_t;
typedef struct
{
long int quot;
long int rem;
} ldiv_t;
# 140 /usr/include/stdlib.h 3 4
extern size_t __ctype_get_mb_cur_max (void) __attribute__ ((__nothrow__)) ;
extern double atof (__const char *__nptr)
__attribute__ ((__nothrow__)) __attribute__ ((__pure__)) __attribute__
((__nonnull__ (1))) ;
extern int atoi (__const char *__nptr)
__attribute__ ((__nothrow__)) __attribute__ ((__pure__)) __attribute__
((__nonnull__ (1))) ;
extern long int atol (__const char *__nptr)
__attribute__ ((__nothrow__)) __attribute__ ((__pure__)) __attribute__
((__nonnull__ (1))) ;
__extension__ extern long long int atoll (__const char *__nptr)
__attribute__ ((__nothrow__)) __attribute__ ((__pure__)) __attribute__
((__nonnull__ (1))) ;
extern double strtod (__const char *__restrict __nptr,
char **__restrict __endptr)
__attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ;
# 182 /usr/include/stdlib.h 3 4
extern long int strtol (__const char *__restrict __nptr,
char **__restrict __endptr, int __base)
__attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ;
extern unsigned long int strtoul (__const char *__restrict __nptr,
char **__restrict __endptr, int __base)
__attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ;
__extension__
extern long long int strtoq (__const char *__restrict __nptr,
char **__restrict __endptr, int __base)
__attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ;
__extension__
extern unsigned long long int strtouq (__const char *__restrict __nptr,
char **__restrict __endptr, int __base)
__attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ;
__extension__
extern long long int strtoll (__const char *__restrict __nptr,
char **__restrict __endptr, int __base)
__attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ;
__extension__
extern unsigned long long int strtoull (__const char *__restrict __nptr,
char **__restrict __endptr, int __base)
__attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ;
# 311 /usr/include/stdlib.h 3 4
extern char *l64a (long int __n) __attribute__ ((__nothrow__)) ;
extern long int a64l (__const char *__s)
__attribute__ ((__nothrow__)) __attribute__ ((__pure__)) __attribute__
((__nonnull__ (1))) ;
# 1 /usr/include/sys/types.h 1 3 4
# 29 /usr/include/sys/types.h 3 4
# 1
Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
Jan Kiszka wrote:
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
My compiler still complains about undefined 'y0' in the enabled case.
I'll try to dig into a different direction now: Automatic generation
during build. This is what the kernel does as well when the preprocessor
gives up. Would even save the DECLARE and should make everyone happy.
No, please nothing like that.
Because ... ?
BTW, I'll extend the prepare stage. Defining the proper dependencies for
build-time generation gets too hairy.
This one works for me:
#include stdlib.h
#include stdio.h
#define __name2(a, b) a ## b
#define name2(a, b) __name2(a, b)
#define DECLARE_ASSERT_SYMBOL(sym) \
static const int XENO_OPT_DEBUG_##sym = 0; \
static const int CONFIG_XENO_OPT_DEBUG_##sym##0 = 0
#define XENO_DEBUG(sym) \
(name2(CONFIG_XENO_OPT_DEBUG_##sym,0) XENO_OPT_DEBUG_##sym)
#define XENO_ASSERT(subsystem,cond,action) do { \
if (unlikely(XENO_DEBUG(subsystem) !(cond))) { \
xnarch_trace_panic_freeze(); \
xnlogerr(assertion failed at %s:%d (%s)\n, __FILE__, __LINE__,
(#cond)); \
xnarch_trace_panic_dump(); \
action; \
} \
} while(0)
DECLARE_ASSERT_SYMBOL(NUCLEUS);
int main(void)
{
if (XENO_DEBUG(NUCLEUS))
printf(Hello\n);
}
Please try and send me the result of pre-processing if
it does not work for you.
Find it attached.
It looks like you are defining CONFIG_XENO_OPT_DEBUG_NUCLEUS to be y
instead of 1.
Right, my bad.
Works now, just leaving a trace when optimization is off.
I believe the current approach would have the same problem. And in any
Nope, it's a pure preprocessor approach.
When you use if(XENO_DEBUG(foo)) you are using the compiler, not the
preprocessor.
To see if there is a difference, you should try #if XENO_DEBUG(foo). And
from what I see, in that case I do not have any trace of anything generated.
diff --git a/scripts/prepare-kernel.sh b/scripts/prepare-kernel.sh
index 24b1f17..d8038e0 100755
Please let me know what precisely you dislike in this approach.
You have to re-run prepare-kernel when you modify the source.
Normally, you have to anyway as you add some header or some other source
file during this process.
Not necessarily.
I do not like dynamic sources generation. Especially when we have a
working solution based on C pre-processor macros.
My prepare-kernel approach also detects stall debug stuff: The uitron
use of XENO_DEBUG is not backed by any config option.
The current candidate would also detect the missing
DECLARE_ASSERT_SYMBOL in that case.
--
Gilles.
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
