* [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs.
@ 2010-04-19 13:58 Gilles Chanteperdrix
2010-04-19 14:00 ` [Xenomai-core] [PATCH] debug: fix direct references to CONFIG_XENO_OPT_DEBUG_* Gilles Chanteperdrix
` (3 more replies)
0 siblings, 4 replies; 38+ messages in thread
From: Gilles Chanteperdrix @ 2010-04-19 13:58 UTC (permalink / raw)
To: xenomai-core
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
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.
--
Gilles.
^ permalink raw reply [flat|nested] 38+ messages in thread* [Xenomai-core] [PATCH] debug: fix direct references to CONFIG_XENO_OPT_DEBUG_* 2010-04-19 13:58 [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs Gilles Chanteperdrix @ 2010-04-19 14:00 ` Gilles Chanteperdrix 2010-04-19 14:05 ` [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs Gilles Chanteperdrix ` (2 subsequent siblings) 3 siblings, 0 replies; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 14:00 UTC (permalink / raw) To: Xenomai core We have some #ifdef CONFIG_XENO_OPT_DEBUG_FOO, it is incompatible with XENO_DEBUG(FOO), XENO_BUGON(FOO, ...) etc... So, this commit: - replaces these #ifdef with #if XENO_DEBUG(FOO) - moves all the piece of code like: to include/nucleus/assert.h, this avoids some code duplication, and allows us to treat as an error remaining references to CONFIG_XENO_OPT_DEBUG_FOO in other files than assert.h. --- include/asm-generic/system.h | 10 ++---- include/native/types.h | 6 +--- include/nucleus/assert.h | 50 ++++++++++++++++++++++++++--- include/nucleus/bheap.h | 8 +--- include/nucleus/heap.h | 4 -- include/nucleus/pod.h | 8 ++-- include/nucleus/queue.h | 4 -- include/nucleus/sched-sporadic.h | 4 -- include/nucleus/sched.h | 6 +--- include/nucleus/synch.h | 6 ++-- include/nucleus/timer.h | 4 -- include/psos+/ppd.h | 6 +--- include/rtdm/rtdm_driver.h | 4 -- include/vxworks/ppd.h | 6 +--- ksrc/nucleus/bufd.c | 6 +--- ksrc/nucleus/pod.c | 64 ++++++++++++++++++-------------------- ksrc/nucleus/registry.c | 6 +--- ksrc/nucleus/sched.c | 12 ++----- ksrc/nucleus/shadow.c | 50 +++++++++++++---------------- ksrc/nucleus/synch.c | 24 ++++++-------- ksrc/skins/posix/internal.h | 6 +--- ksrc/skins/rtdm/internal.h | 4 -- ksrc/skins/uitron/ppd.h | 6 +--- src/skins/native/mutex.c | 2 +- 24 files changed, 133 insertions(+), 173 deletions(-) diff --git a/include/asm-generic/system.h b/include/asm-generic/system.h index a2c8fb9..6a255c0 100644 --- a/include/asm-generic/system.h +++ b/include/asm-generic/system.h @@ -44,10 +44,6 @@ /* debug support */ #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_XNLOCK -#define CONFIG_XENO_OPT_DEBUG_XNLOCK 0 -#endif - #ifdef __cplusplus extern "C" { #endif @@ -452,9 +448,9 @@ static inline int xnarch_remap_io_page_range(struct file *filp, unsigned long size, pgprot_t prot) { - return wrap_remap_io_page_range(vma, from, to, size, - wrap_phys_mem_prot(filp, (to) >> PAGE_SHIFT, - size, prot)); + return wrap_remap_io_page_range(vma, from, to, size, + wrap_phys_mem_prot(filp, (to) >> PAGE_SHIFT, + size, prot)); } static inline int xnarch_remap_kmem_page_range(struct vm_area_struct *vma, diff --git a/include/native/types.h b/include/native/types.h index 0dd721f..daeedff 100644 --- a/include/native/types.h +++ b/include/native/types.h @@ -2,7 +2,7 @@ * @file * This file is part of the Xenomai project. * - * @note Copyright (C) 2004 Philippe Gerum <rpm@xenomai.org> + * @note Copyright (C) 2004 Philippe Gerum <rpm@xenomai.org> * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -32,10 +32,6 @@ #if defined(__KERNEL__) || defined(__XENO_SIM__) -#ifndef CONFIG_XENO_OPT_DEBUG_NATIVE -#define CONFIG_XENO_OPT_DEBUG_NATIVE 0 -#endif - typedef xnticks_t RTIME; typedef xnsticks_t SRTIME; diff --git a/include/nucleus/assert.h b/include/nucleus/assert.h index 9cb88af..62bc329 100644 --- a/include/nucleus/assert.h +++ b/include/nucleus/assert.h @@ -26,16 +26,56 @@ #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; \ + xnarch_trace_panic_freeze(); \ + xnlogerr("assertion failed at %s:%d (%s)\n", __FILE__, __LINE__, (#cond)); \ + xnarch_trace_panic_dump(); \ + action; \ } \ } while(0) #define XENO_BUGON(subsystem,cond) do { \ if (unlikely(CONFIG_XENO_OPT_DEBUG_##subsystem > 0 && (cond))) \ - xnpod_fatal("bug at %s:%d (%s)", __FILE__, __LINE__, (#cond)); \ + xnpod_fatal("bug at %s:%d (%s)", __FILE__, __LINE__, (#cond)); \ } while(0) +#ifndef CONFIG_XENO_OPT_DEBUG_NATIVE +#define CONFIG_XENO_OPT_DEBUG_NATIVE 0 +#endif /* CONFIG_XENO_OPT_DEBUG_NATIVE */ +#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS +#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 +#endif /* CONFIG_XENO_OPT_DEBUG_NUCLEUS */ +#ifndef CONFIG_XENO_OPT_DEBUG_POSIX +#define CONFIG_XENO_OPT_DEBUG_POSIX 0 +#endif /* CONFIG_XENO_OPT_DEBUG_POSIX */ +#ifndef CONFIG_XENO_OPT_DEBUG_PSOS +#define CONFIG_XENO_OPT_DEBUG_PSOS 0 +#endif /* CONFIG_XENO_OPT_DEBUG_PSOS */ +#ifndef CONFIG_XENO_OPT_DEBUG_QUEUES +#define CONFIG_XENO_OPT_DEBUG_QUEUES 0 +#endif /* CONFIG_XENO_OPT_DEBUG_QUEUES */ +#ifndef CONFIG_XENO_OPT_DEBUG_REGISTRY +#define CONFIG_XENO_OPT_DEBUG_REGISTRY 0 +#endif /* CONFIG_XENO_OPT_DEBUG_REGISTRY */ +#ifndef CONFIG_XENO_OPT_DEBUG_RTDM +#define CONFIG_XENO_OPT_DEBUG_RTDM 0 +#endif /* CONFIG_XENO_OPT_DEBUG_RTDM */ +#ifndef CONFIG_XENO_OPT_DEBUG_RTDM_APPL +#define CONFIG_XENO_OPT_DEBUG_RTDM_APPL 0 +#endif /* CONFIG_XENO_OPT_DEBUG_RTDM_APPL */ +#ifndef CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX +#define CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX 0 +#endif /* CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX */ +#ifndef CONFIG_XENO_OPT_DEBUG_TIMERS +#define CONFIG_XENO_OPT_DEBUG_TIMERS 0 +#endif /* CONFIG_XENO_OPT_DEBUG_TIMERS */ +#ifndef CONFIG_XENO_OPT_DEBUG_UITRON +#define CONFIG_XENO_OPT_DEBUG_UITRON 0 +#endif /* CONFIG_XENO_OPT_DEBUG_UITRON */ +#ifndef CONFIG_XENO_OPT_DEBUG_VXWORKS +#define CONFIG_XENO_OPT_DEBUG_VXWORKS 0 +#endif /* CONFIG_XENO_OPT_DEBUG_VXWORKS */ +#ifndef CONFIG_XENO_OPT_DEBUG_XNLOCK +#define CONFIG_XENO_OPT_DEBUG_XNLOCK 0 +#endif /* CONFIG_XENO_OPT_DEBUG_XNLOCK */ + #endif /* !_XENO_NUCLEUS_ASSERT_H */ diff --git a/include/nucleus/bheap.h b/include/nucleus/bheap.h index 8b7798e..58ea93f 100644 --- a/include/nucleus/bheap.h +++ b/include/nucleus/bheap.h @@ -25,10 +25,6 @@ /* debug support */ #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_QUEUES -#define CONFIG_XENO_OPT_DEBUG_QUEUES 0 -#endif - /* Priority queue implementation, using a binary heap. */ typedef unsigned long long bheap_key_t; @@ -44,8 +40,8 @@ typedef struct bheaph { #define bheaph_prio(holder) ((holder)->prio) #define bheaph_pos(holder) ((holder)->pos) #define bheaph_lt(h1, h2) ((long long) ((h1)->key - (h2)->key) < 0 || \ - ((h1)->key == (h2)->key && \ - (h1)->prio > (h2)->prio)) + ((h1)->key == (h2)->key && \ + (h1)->prio > (h2)->prio)) typedef struct bheap { unsigned sz; diff --git a/include/nucleus/heap.h b/include/nucleus/heap.h index 77fefa6..fecdb79 100644 --- a/include/nucleus/heap.h +++ b/include/nucleus/heap.h @@ -46,10 +46,6 @@ #if defined(__KERNEL__) || defined(__XENO_SIM__) -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - #define XNHEAP_PAGE_SIZE 512 /* A reasonable value for the xnheap page size */ #define XNHEAP_PAGE_MASK (~(XNHEAP_PAGE_SIZE-1)) #define XNHEAP_PAGE_ALIGN(addr) (((addr)+XNHEAP_PAGE_SIZE-1)&XNHEAP_PAGE_MASK) diff --git a/include/nucleus/pod.h b/include/nucleus/pod.h index e652a1e..8831fb5 100644 --- a/include/nucleus/pod.h +++ b/include/nucleus/pod.h @@ -58,7 +58,7 @@ struct xnsynch; -/*! +/*! * \brief Real-time pod descriptor. * * The source of all Xenomai magic. @@ -276,14 +276,14 @@ static inline void xnpod_schedule(void) * context is active, or if we are caught in the middle of a * unlocked context switch. */ -#ifdef CONFIG_XENO_OPT_DEBUG_NUCLEUS +#if XENO_DEBUG(NUCLEUS) if (testbits(sched->status, XNKCOUT|XNINIRQ|XNSWLOCK)) return; -#else /* !CONFIG_XENO_OPT_DEBUG_NUCLEUS */ +#else /* !XENO_DEBUG(NUCLEUS) */ if (testbits(sched->status, XNKCOUT|XNINIRQ|XNSWLOCK|XNRESCHED) != XNRESCHED) return; -#endif /* !CONFIG_XENO_OPT_DEBUG_NUCLEUS */ +#endif /* !XENO_DEBUG(NUCLEUS) */ __xnpod_schedule(sched); } diff --git a/include/nucleus/queue.h b/include/nucleus/queue.h index e243f2f..15ec537 100644 --- a/include/nucleus/queue.h +++ b/include/nucleus/queue.h @@ -24,10 +24,6 @@ #include <nucleus/types.h> #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_QUEUES -#define CONFIG_XENO_OPT_DEBUG_QUEUES 0 -#endif - /* Basic element holder */ typedef struct xnholder { diff --git a/include/nucleus/sched-sporadic.h b/include/nucleus/sched-sporadic.h index ecebc55..dfeec76 100644 --- a/include/nucleus/sched-sporadic.h +++ b/include/nucleus/sched-sporadic.h @@ -29,10 +29,6 @@ #ifdef CONFIG_XENO_OPT_SCHED_SPORADIC -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - #include <nucleus/heap.h> extern struct xnsched_class xnsched_class_sporadic; diff --git a/include/nucleus/sched.h b/include/nucleus/sched.h index c96d65d..05dd4b1 100644 --- a/include/nucleus/sched.h +++ b/include/nucleus/sched.h @@ -36,10 +36,6 @@ #include <nucleus/sched-tp.h> #include <nucleus/sched-sporadic.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - /* Sched status flags */ #define XNKCOUT 0x80000000 /* Sched callout context */ #define XNHTICK 0x40000000 /* Host tick pending */ @@ -57,7 +53,7 @@ struct xnsched_rt { #endif /* CONFIG_XENO_OPT_PRIOCPL */ }; -/*! +/*! * \brief Scheduling information structure. */ diff --git a/include/nucleus/synch.h b/include/nucleus/synch.h index bd9dbbc..d7edbc6 100644 --- a/include/nucleus/synch.h +++ b/include/nucleus/synch.h @@ -157,14 +157,14 @@ typedef struct xnsynch { extern "C" { #endif -#ifdef CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX +#if XENO_DEBUG(SYNCH_RELAX) void xnsynch_detect_relaxed_owner(struct xnsynch *synch, struct xnthread *sleeper); void xnsynch_detect_claimed_relax(struct xnthread *owner); -#else /* !CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX */ +#else /* !XENO_DEBUG(SYNCH_RELAX) */ static inline void xnsynch_detect_relaxed_owner(struct xnsynch *synch, struct xnthread *sleeper) @@ -175,7 +175,7 @@ static inline void xnsynch_detect_claimed_relax(struct xnthread *owner) { } -#endif /* !CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX */ +#endif /* !XENO_DEBUG(SYNCH_RELAX) */ void xnsynch_init(struct xnsynch *synch, xnflags_t flags, xnarch_atomic_t *fastlock); diff --git a/include/nucleus/timer.h b/include/nucleus/timer.h index ef05822..f2e0b19 100644 --- a/include/nucleus/timer.h +++ b/include/nucleus/timer.h @@ -28,10 +28,6 @@ #if defined(__KERNEL__) || defined(__XENO_SIM__) -#ifndef CONFIG_XENO_OPT_DEBUG_TIMERS -#define CONFIG_XENO_OPT_DEBUG_TIMERS 0 -#endif - #define XNTIMER_WHEELSIZE 64 #define XNTIMER_WHEELMASK (XNTIMER_WHEELSIZE - 1) diff --git a/include/psos+/ppd.h b/include/psos+/ppd.h index 09ea13f..82f7834 100644 --- a/include/psos+/ppd.h +++ b/include/psos+/ppd.h @@ -2,7 +2,7 @@ * @file * This file is part of the Xenomai project. * - * @note Copyright (C) 2007 Philippe Gerum <rpm@xenomai.org> + * @note Copyright (C) 2007 Philippe Gerum <rpm@xenomai.org> * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -25,10 +25,6 @@ #include <nucleus/pod.h> #include <nucleus/ppd.h> -#ifndef CONFIG_XENO_OPT_DEBUG_PSOS -#define CONFIG_XENO_OPT_DEBUG_PSOS 0 -#endif - typedef struct psos_resource_holder { xnshadow_ppd_t ppd; diff --git a/include/rtdm/rtdm_driver.h b/include/rtdm/rtdm_driver.h index 0fc1496..0d11666 100644 --- a/include/rtdm/rtdm_driver.h +++ b/include/rtdm/rtdm_driver.h @@ -47,10 +47,6 @@ #include <asm-generic/xenomai/pci_ids.h> #endif /* CONFIG_PCI */ -#ifndef CONFIG_XENO_OPT_DEBUG_RTDM -#define CONFIG_XENO_OPT_DEBUG_RTDM 0 -#endif - struct rtdm_dev_context; typedef struct xnselector rtdm_selector_t; enum rtdm_selecttype; diff --git a/include/vxworks/ppd.h b/include/vxworks/ppd.h index 245c9b7..bae6fa1 100644 --- a/include/vxworks/ppd.h +++ b/include/vxworks/ppd.h @@ -2,7 +2,7 @@ * @file * This file is part of the Xenomai project. * - * @note Copyright (C) 2007 Philippe Gerum <rpm@xenomai.org> + * @note Copyright (C) 2007 Philippe Gerum <rpm@xenomai.org> * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -25,10 +25,6 @@ #include <nucleus/pod.h> #include <nucleus/ppd.h> -#ifndef CONFIG_XENO_OPT_DEBUG_VXWORKS -#define CONFIG_XENO_OPT_DEBUG_VXWORKS 0 -#endif - typedef struct wind_resource_holder { xnshadow_ppd_t ppd; diff --git a/ksrc/nucleus/bufd.c b/ksrc/nucleus/bufd.c index 8dd655c..d7a7b00 100644 --- a/ksrc/nucleus/bufd.c +++ b/ksrc/nucleus/bufd.c @@ -58,7 +58,7 @@ * * ret = this_may_fail(); * if (ret) - * xnbufd_invalidate(bufd); + * xnbufd_invalidate(bufd); * * return ret; * } @@ -146,10 +146,6 @@ #include <nucleus/bufd.h> #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - #ifdef CONFIG_XENO_OPT_PERVASIVE #include <asm/xenomai/syscall.h> diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c index 93713f2..3ffd548 100644 --- a/ksrc/nucleus/pod.c +++ b/ksrc/nucleus/pod.c @@ -46,10 +46,6 @@ #include <nucleus/select.h> #include <asm/xenomai/bits/pod.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - /* * NOTE: We need to initialize the globals; remember that this code * also runs over the simulator in user-space. @@ -318,7 +314,7 @@ static void xnpod_flush_stackpool(xnheap_t *heap, } #endif -/*! +/*! * \fn int xnpod_init(void) * \brief Initialize the core pod. * @@ -438,7 +434,7 @@ int xnpod_init(void) } EXPORT_SYMBOL_GPL(xnpod_init); -/*! +/*! * \fn void xnpod_shutdown(int xtype) * \brief Shutdown the current pod. * @@ -543,7 +539,7 @@ void xnpod_fire_callouts(xnqueue_t *hookq, xnthread_t *thread) __clrbits(sched->status, XNKCOUT); } -/*! +/*! * \fn void xnpod_init_thread(struct xnthread *thread,const struct xnthread_init_attr *attr,struct xnsched_class *sched_class,const union xnsched_policy_param *sched_param) * \brief Initialize a new thread. * @@ -665,7 +661,7 @@ int xnpod_init_thread(struct xnthread *thread, } EXPORT_SYMBOL_GPL(xnpod_init_thread); -/*! +/*! * \fn int xnpod_start_thread(struct xnthread *thread,const struct xnthread_start_attr *attr) * \brief Initial start of a newly created thread. * @@ -824,7 +820,7 @@ unlock_and_exit: } EXPORT_SYMBOL_GPL(xnpod_start_thread); -/*! +/*! * @internal * \fn void __xnpod_reset_thread(struct xnthread *thread, int unlock) * \brief Reset the thread. @@ -861,7 +857,7 @@ void __xnpod_reset_thread(struct xnthread *thread) } } -/*! +/*! * \fn void xnpod_stop_thread(xnthread_t *thread) * * \brief Stop a thread. @@ -907,7 +903,7 @@ void xnpod_stop_thread(struct xnthread *thread) } EXPORT_SYMBOL_GPL(xnpod_stop_thread); -/*! +/*! * \fn void xnpod_restart_thread(xnthread_t *thread) * * \brief Restart a thread. @@ -968,7 +964,7 @@ void xnpod_restart_thread(xnthread_t *thread) } EXPORT_SYMBOL_GPL(xnpod_restart_thread); -/*! +/*! * \fn void xnpod_set_thread_mode(xnthread_t *thread,xnflags_t clrmask,xnflags_t setmask) * \brief Change a thread's control mode. * @@ -1043,7 +1039,7 @@ xnflags_t xnpod_set_thread_mode(xnthread_t *thread, } EXPORT_SYMBOL_GPL(xnpod_set_thread_mode); -/*! +/*! * \fn void xnpod_delete_thread(xnthread_t *thread) * * \brief Delete a thread. @@ -1247,7 +1243,7 @@ void xnpod_delete_thread(xnthread_t *thread) } EXPORT_SYMBOL_GPL(xnpod_delete_thread); -/*! +/*! * \fn void xnpod_abort_thread(xnthread_t *thread) * * \brief Abort a thread. @@ -1408,9 +1404,9 @@ void xnpod_suspend_thread(xnthread_t *thread, xnflags_t mask, * the wakeup call pending for that task in the root * context, to collect and act upon the pending Linux * signal. - */ - if ((mask & XNRELAX) == 0 - && (xnthread_sigpending(thread) + */ + if ((mask & XNRELAX) == 0 + && (xnthread_sigpending(thread) || xnthread_test_info(thread, XNKICKED))) { xnthread_clear_info(thread, XNRMID | XNTIMEO); xnthread_set_info(thread, XNBREAK); @@ -1495,7 +1491,7 @@ void xnpod_suspend_thread(xnthread_t *thread, xnflags_t mask, * those. So there is no need to deal specifically with the * relax+suspend issue when the about to be suspended thread * is current, since it must not be relaxed anyway. - * + * * - among all blocking bits (XNTHREAD_BLOCK_BITS), only * XNSUSP, XNDELAY, XNDORMANT and XNHELD may be applied by the * current thread to a non-current thread. XNPEND is always @@ -1900,16 +1896,16 @@ unlock_and_exit: return ret; } -/** +/** * \fn int xnpod_migrate_thread(int cpu) * * \brief Migrate the current thread. * * This call makes the current thread migrate to another CPU if its * affinity allows it. - * + * * @param cpu The destination CPU. - * + * * @retval 0 if the thread could migrate ; * @retval -EPERM if the calling context is asynchronous, or the * current thread affinity forbids this migration ; @@ -1971,7 +1967,7 @@ int xnpod_migrate_thread(int cpu) } EXPORT_SYMBOL_GPL(xnpod_migrate_thread); -/*! +/*! * @internal * \fn void xnpod_dispatch_signals(void) * \brief Deliver pending asynchronous signals to the running thread. @@ -2079,7 +2075,7 @@ static inline void xnpod_switch_to(xnsched_t *sched, xnthread_archtcb(next)); } -/*! +/*! * \fn void xnpod_schedule(void) * \brief Rescheduling procedure entry point. * @@ -2133,7 +2129,7 @@ static inline void xnpod_switch_to(xnsched_t *sched, * context switch has taken place. This behaviour can be disabled by * setting the XNASDI flag in the thread's status mask by calling * xnpod_set_thread_mode(). - * + * * Environments: * * This service can be called from: @@ -2149,7 +2145,7 @@ static inline void xnpod_switch_to(xnsched_t *sched, static inline int __xnpod_test_resched(struct xnsched *sched) { int cpu = xnsched_cpu(sched), resched; - + resched = xnarch_cpu_isset(cpu, sched->resched); xnarch_cpu_clear(cpu, sched->resched); #ifdef CONFIG_SMP @@ -2181,10 +2177,10 @@ void __xnpod_schedule(struct xnsched *sched) xnthread_current_priority(curr)); need_resched = __xnpod_test_resched(sched); -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS +#if !XENO_DEBUG(NUCLEUS) if (!need_resched) goto signal_unlock_and_exit; -#endif /* !CONFIG_XENO_OPT_DEBUG_NUCLEUS */ +#endif /* !XENO_DEBUG(NUCLEUS) */ zombie = xnthread_test_state(curr, XNZOMBIE); next = xnsched_pick_next(sched); @@ -2293,7 +2289,7 @@ void __xnpod_schedule(struct xnsched *sched) { spl_t ignored; - /* Shadow on entry and root without shadow extension on exit? + /* Shadow on entry and root without shadow extension on exit? Mmmm... This must be the user-space mate of a deleted real-time shadow we've just rescheduled in the Linux domain to have it exit properly. Reap it now. */ @@ -2348,7 +2344,7 @@ void xnpod_unlock_sched(void) } EXPORT_SYMBOL_GPL(xnpod_unlock_sched); -/*! +/*! * \fn int xnpod_add_hook(int type,void (*routine)(xnthread_t *)) * \brief Install a nucleus hook. * @@ -2441,7 +2437,7 @@ int xnpod_add_hook(int type, void (*routine) (xnthread_t *)) } EXPORT_SYMBOL_GPL(xnpod_add_hook); -/*! +/*! * \fn int xnpod_remove_hook(int type,void (*routine)(xnthread_t *)) * \brief Remove a nucleus hook. * @@ -2518,7 +2514,7 @@ int xnpod_remove_hook(int type, void (*routine) (xnthread_t *)) } EXPORT_SYMBOL_GPL(xnpod_remove_hook); -/*! +/*! * \fn void xnpod_trap_fault(xnarch_fltinfo_t *fltinfo); * \brief Default fault handler. * @@ -2609,7 +2605,7 @@ int xnpod_trap_fault(xnarch_fltinfo_t *fltinfo) } EXPORT_SYMBOL_GPL(xnpod_trap_fault); -/*! +/*! * \fn int xnpod_enable_timesource(void) * \brief Activate the core time source. * @@ -2750,7 +2746,7 @@ int xnpod_enable_timesource(void) } EXPORT_SYMBOL_GPL(xnpod_enable_timesource); -/*! +/*! * \fn void xnpod_disable_timesource(void) * \brief Stop the core time source. * @@ -3055,7 +3051,7 @@ int xnpod_set_thread_tslice(struct xnthread *thread, xnticks_t quantum) unsigned long oldmode; int aperiodic; spl_t s; - + if (thread->base_class->sched_tick == NULL) return -EINVAL; diff --git a/ksrc/nucleus/registry.c b/ksrc/nucleus/registry.c index 9958dd3..8fb42fb 100644 --- a/ksrc/nucleus/registry.c +++ b/ksrc/nucleus/registry.c @@ -2,7 +2,7 @@ * @file * This file is part of the Xenomai project. * - * @note Copyright (C) 2004 Philippe Gerum <rpm@xenomai.org> + * @note Copyright (C) 2004 Philippe Gerum <rpm@xenomai.org> * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -42,10 +42,6 @@ #include <nucleus/thread.h> #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_REGISTRY -#define CONFIG_XENO_OPT_DEBUG_REGISTRY 0 -#endif - static xnobject_t *registry_obj_slots; static xnqueue_t registry_obj_freeq; /* Free objects. */ diff --git a/ksrc/nucleus/sched.c b/ksrc/nucleus/sched.c index 4ec0013..0b737a3 100644 --- a/ksrc/nucleus/sched.c +++ b/ksrc/nucleus/sched.c @@ -66,7 +66,7 @@ static u_long wd_timeout_arg = CONFIG_XENO_OPT_WATCHDOG_TIMEOUT; module_param_named(watchdog_timeout, wd_timeout_arg, ulong, 0644); MODULE_PARM_DESC(watchdog_timeout, "Watchdog timeout (s)"); -/*! +/*! * @internal * \fn void xnsched_watchdog_handler(struct xntimer *timer) * \brief Process watchdog ticks. @@ -307,7 +307,7 @@ struct xnthread *xnsched_peek_rpi(struct xnsched *sched) #endif /* CONFIG_XENO_OPT_SCHED_CLASSES */ } -/*! +/*! * @internal * \fn void xnsched_renice_root(struct xnsched *sched, struct xnthread *target) * \brief Change the root thread priority. @@ -529,10 +529,6 @@ void xnsched_migrate_passive(struct xnthread *thread, struct xnsched *sched) #ifdef CONFIG_XENO_OPT_SCALABLE_SCHED -#ifndef CONFIG_XENO_OPT_DEBUG_QUEUES -#define CONFIG_XENO_OPT_DEBUG_QUEUES 0 -#endif - void initmlq(struct xnsched_mlq *q, int loprio, int hiprio) { int prio; @@ -546,7 +542,7 @@ void initmlq(struct xnsched_mlq *q, int loprio, int hiprio) for (prio = 0; prio < XNSCHED_MLQ_LEVELS; prio++) initq(&q->queue[prio]); - XENO_ASSERT(QUEUES, + XENO_ASSERT(QUEUES, hiprio - loprio + 1 < XNSCHED_MLQ_LEVELS, xnpod_fatal("priority range [%d..%d] is beyond multi-level " "queue indexing capabilities", @@ -706,7 +702,7 @@ struct sched_seq_iterator { int cprio; int dnprio; int periodic; - xnticks_t timeout; + xnticks_t timeout; xnflags_t state; } sched_info[1]; }; diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c index 539d952..872c37f 100644 --- a/ksrc/nucleus/shadow.c +++ b/ksrc/nucleus/shadow.c @@ -55,10 +55,6 @@ #include <asm/xenomai/syscall.h> #include <asm/xenomai/bits/shadow.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - static int xn_gid_arg = -1; module_param_named(xenomai_gid, xn_gid_arg, int, 0644); MODULE_PARM_DESC(xenomai_gid, "GID of the group with access to Xenomai services"); @@ -179,7 +175,7 @@ static struct xnthread *rpi_next(struct xnsched *sched, spl_t s) struct xnthread *thread; thread = xnsched_peek_rpi(sched); - while (thread && + while (thread && xnthread_user_task(thread)->state != TASK_RUNNING && !xnthread_test_info(thread, XNATOMIC)) { /* @@ -458,7 +454,7 @@ static inline void rpi_switch(struct task_struct *next_task) next->rpi = sched; xnlock_put_irqrestore(&sched->rpilock, s); xnlock_get_irqsave(&nklock, s); - xnsched_resume_rpi(next); + xnsched_resume_rpi(next); xnlock_put_irqrestore(&nklock, s); } } else if (unlikely(next->rpi != sched)) @@ -500,17 +496,17 @@ void xnshadow_rpi_check(void) struct xnsched *sched = xnpod_current_sched(); struct xnthread *top; spl_t s; - - xnlock_get_irqsave(&sched->rpilock, s); - top = rpi_next(sched, s); - xnlock_put_irqrestore(&sched->rpilock, s); + + xnlock_get_irqsave(&sched->rpilock, s); + top = rpi_next(sched, s); + xnlock_put_irqrestore(&sched->rpilock, s); if (top == NULL && xnsched_root_class(sched) != &xnsched_class_idle) xnsched_renice_root(sched, NULL); } #endif /* CONFIG_SMP */ - + #else #define rpi_p(t) (0) @@ -660,14 +656,14 @@ static inline void ppd_remove_mm(struct mm_struct *mm, xnlock_put_irqrestore(&nklock, s); } -/** +/** * Mark the per-thread per-skin signal condition for the skin whose * muxid is @a muxid. - * + * * @param thread the target thread; * @param muxid the muxid of the skin for which the signal is marked * pending. - * + * * @return whether rescheduling is needed. */ int xnshadow_mark_sig(xnthread_t *thread, unsigned muxid) @@ -687,13 +683,13 @@ int xnshadow_mark_sig(xnthread_t *thread, unsigned muxid) } EXPORT_SYMBOL_GPL(xnshadow_mark_sig); -/** +/** * Clear the per-thrad per-skin signal condition. - * + * * Called with nklock locked irqs off. * * @param thread the target thrad - * @param muxid + * @param muxid */ void xnshadow_clear_sig(xnthread_t *thread, unsigned muxid) { @@ -754,7 +750,7 @@ static inline void handle_rt_signals(xnthread_t *thread, /* In the case when we are going to handle linux signals, do not let the kernel handle the syscall restart to give a chance to the xenomai handlers to be executed. */ - if (__xn_interrupted_p(regs) + if (__xn_interrupted_p(regs) || __xn_reg_rval(regs) == -ERESTARTSYS || __xn_reg_rval(regs) == -ERESTARTNOHAND) __xn_error_return(regs, ((sysflags & __xn_exec_norestart) @@ -902,7 +898,7 @@ static void schedule_linux_call(int type, struct task_struct *p, int arg) rq->req[reqnum].type = type; rq->req[reqnum].task = p; rq->req[reqnum].arg = arg; - + splexit(s); rthal_apc_schedule(lostage_apc); @@ -979,7 +975,7 @@ static int gatekeeper_thread(void *data) return 0; } -/*! +/*! * @internal * \fn int xnshadow_harden(void); * \brief Migrate a Linux task to the Xenomai domain. @@ -1111,7 +1107,7 @@ redo: } EXPORT_SYMBOL_GPL(xnshadow_harden); -/*! +/*! * @internal * \fn void xnshadow_relax(int notify, int reason); * \brief Switch a shadow thread back to the Linux domain. @@ -1280,7 +1276,7 @@ void xnshadow_exit(void) * * This service can be called from: * - * - Regular user-space process. + * - Regular user-space process. * * Rescheduling: always. * @@ -1365,10 +1361,10 @@ int xnshadow_map(xnthread_t *thread, xncompletion_t __user *u_completion, */ xnshadow_renice(thread); - /* + /* * We still have the XNDORMANT bit set, so we can't - * link to the RPI queue which only links _runnable_ - * relaxed shadow. + * link to the RPI queue which only links _runnable_ + * relaxed shadow. */ xnshadow_signal_completion(u_completion, 0); return 0; @@ -1994,7 +1990,7 @@ static xnsysent_t __systab[] = { [__xn_sys_current] = {&xnshadow_sys_current, __xn_exec_any}, [__xn_sys_current_info] = {&xnshadow_sys_current_info, __xn_exec_shadow}, - [__xn_sys_get_next_sigs] = + [__xn_sys_get_next_sigs] = {&xnshadow_sys_get_next_sigs, __xn_exec_conforming}, [__xn_sys_drop_u_mode] = {&xnshadow_sys_drop_u_mode, __xn_exec_shadow}, @@ -2748,7 +2744,7 @@ EXPORT_SYMBOL_GPL(xnshadow_unregister_interface); * * @return the per-process data if the current context is a user-space process; * @return NULL otherwise. - * + * */ xnshadow_ppd_t *xnshadow_ppd_get(unsigned muxid) { diff --git a/ksrc/nucleus/synch.c b/ksrc/nucleus/synch.c index 619929e..d57f7cd 100644 --- a/ksrc/nucleus/synch.c +++ b/ksrc/nucleus/synch.c @@ -36,14 +36,10 @@ #include <nucleus/thread.h> #include <nucleus/module.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - #define w_bprio(t) xnsched_weighted_bprio(t) #define w_cprio(t) xnsched_weighted_cprio(t) -/*! +/*! * \fn void xnsynch_init(struct xnsynch *synch, xnflags_t flags, * xnarch_atomic_t *fastlock) * @@ -360,7 +356,7 @@ static void xnsynch_renice_thread(struct xnthread *thread, #endif /* CONFIG_XENO_OPT_PERVASIVE */ } -/*! +/*! * \fn xnflags_t xnsynch_acquire(struct xnsynch *synch, xnticks_t timeout, * xntmode_t timeout_mode); * \brief Acquire the ownership of a synchronization object. @@ -561,7 +557,7 @@ xnflags_t xnsynch_acquire(struct xnsynch *synch, xnticks_t timeout, } EXPORT_SYMBOL_GPL(xnsynch_acquire); -/*! +/*! * @internal * \fn void xnsynch_clear_boost(struct xnsynch *synch, struct xnthread *owner); * \brief Clear the priority boost. @@ -609,7 +605,7 @@ static void xnsynch_clear_boost(struct xnsynch *synch, xnsynch_renice_thread(owner, target); } -/*! +/*! * @internal * \fn void xnsynch_requeue_sleeper(struct xnthread *thread); * \brief Change a sleeper's priority. @@ -761,7 +757,7 @@ struct xnthread *xnsynch_release(struct xnsynch *synch) } EXPORT_SYMBOL_GPL(xnsynch_release); -/*! +/*! * \fn struct xnthread *xnsynch_peek_pendq(struct xnsynch *synch); * \brief Access the thread leading a synch object wait queue. * @@ -800,7 +796,7 @@ struct xnthread *xnsynch_peek_pendq(struct xnsynch *synch) } EXPORT_SYMBOL_GPL(xnsynch_peek_pendq); -/*! +/*! * \fn void xnsynch_flush(struct xnsynch *synch, xnflags_t reason); * \brief Unblock all waiters pending on a resource. * @@ -882,7 +878,7 @@ int xnsynch_flush(struct xnsynch *synch, xnflags_t reason) } EXPORT_SYMBOL_GPL(xnsynch_flush); -/*! +/*! * @internal * \fn void xnsynch_forget_sleeper(struct xnthread *thread); * \brief Abort a wait for a resource. @@ -947,7 +943,7 @@ void xnsynch_forget_sleeper(struct xnthread *thread) } EXPORT_SYMBOL_GPL(xnsynch_forget_sleeper); -/*! +/*! * @internal * \fn void xnsynch_release_all_ownerships(struct xnthread *thread); * \brief Release all ownerships. @@ -981,7 +977,7 @@ void xnsynch_release_all_ownerships(struct xnthread *thread) } EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships); -#ifdef CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX +#if XENO_DEBUG(SYNCH_RELAX) /* * Detect when a thread is about to sleep on a synchronization @@ -1025,7 +1021,7 @@ void xnsynch_detect_claimed_relax(struct xnthread *owner) } } -#endif /* CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX */ +#endif /* XENO_DEBUG(SYNCH_RELAX) */ /*@}*/ diff --git a/ksrc/skins/posix/internal.h b/ksrc/skins/posix/internal.h index dcc71e9..46fa547 100644 --- a/ksrc/skins/posix/internal.h +++ b/ksrc/skins/posix/internal.h @@ -28,10 +28,6 @@ /* debug support */ #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_POSIX -#define CONFIG_XENO_OPT_DEBUG_POSIX 0 -#endif - #define PSE51_MAGIC(n) (0x8686##n##n) #define PSE51_ANY_MAGIC PSE51_MAGIC(00) #define PSE51_THREAD_MAGIC PSE51_MAGIC(01) @@ -103,7 +99,7 @@ static inline pse51_queues_t *pse51_queues(void) xnlock_get_irqsave(&nklock, s); ppd = xnshadow_ppd_get(pse51_muxid); - + xnlock_put_irqrestore(&nklock, s); if (!ppd) diff --git a/ksrc/skins/rtdm/internal.h b/ksrc/skins/rtdm/internal.h index 69299f8..64e5b47 100644 --- a/ksrc/skins/rtdm/internal.h +++ b/ksrc/skins/rtdm/internal.h @@ -28,10 +28,6 @@ #include <nucleus/ppd.h> #include <rtdm/rtdm_driver.h> -#ifndef CONFIG_XENO_OPT_DEBUG_RTDM_APPL -#define CONFIG_XENO_OPT_DEBUG_RTDM_APPL 0 -#endif - #define RTDM_FD_MAX CONFIG_XENO_OPT_RTDM_FILDES #define DEF_DEVNAME_HASHTAB_SIZE 256 /* entries in name hash table */ diff --git a/ksrc/skins/uitron/ppd.h b/ksrc/skins/uitron/ppd.h index 07616ec..c851cb9 100644 --- a/ksrc/skins/uitron/ppd.h +++ b/ksrc/skins/uitron/ppd.h @@ -2,7 +2,7 @@ * @file * This file is part of the Xenomai project. * - * @note Copyright (C) 2007 Philippe Gerum <rpm@xenomai.org> + * @note Copyright (C) 2007 Philippe Gerum <rpm@xenomai.org> * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -25,10 +25,6 @@ #include <nucleus/pod.h> #include <nucleus/ppd.h> -#ifndef CONFIG_XENO_OPT_DEBUG_UITRON -#define CONFIG_XENO_OPT_DEBUG_UITRON 0 -#endif - typedef struct ui_resource_holder { xnshadow_ppd_t ppd; diff --git a/src/skins/native/mutex.c b/src/skins/native/mutex.c index 98844ef..55d502f 100644 --- a/src/skins/native/mutex.c +++ b/src/skins/native/mutex.c @@ -103,7 +103,7 @@ static int rt_mutex_acquire_inner(RT_MUTEX *mutex, RTIME timeout, xntmode_t mode * mutex state consistent. * * We make no efforts to migrate or warn here. There is - * CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX to catch such bugs. + * XENO_DEBUG(SYNCH_RELAX) to catch such bugs. */ if (mutex->lockcnt == UINT_MAX) return -EAGAIN; -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 13:58 [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs Gilles Chanteperdrix 2010-04-19 14:00 ` [Xenomai-core] [PATCH] debug: fix direct references to CONFIG_XENO_OPT_DEBUG_* Gilles Chanteperdrix @ 2010-04-19 14:05 ` Gilles Chanteperdrix 2010-04-19 14:47 ` Jan Kiszka 2010-04-19 15:06 ` Philippe Gerum 3 siblings, 0 replies; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 14:05 UTC (permalink / raw) To: xenomai-core Gilles Chanteperdrix wrote: > (...) > So, a patch follows which: Sorry, here is a patch without the whitespace changes. diff --git a/include/asm-generic/system.h b/include/asm-generic/system.h index a2c8fb9..6a255c0 100644 --- a/include/asm-generic/system.h +++ b/include/asm-generic/system.h @@ -44,10 +44,6 @@ /* debug support */ #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_XNLOCK -#define CONFIG_XENO_OPT_DEBUG_XNLOCK 0 -#endif - #ifdef __cplusplus extern "C" { #endif diff --git a/include/native/types.h b/include/native/types.h index 0dd721f..daeedff 100644 --- a/include/native/types.h +++ b/include/native/types.h @@ -32,10 +32,6 @@ #if defined(__KERNEL__) || defined(__XENO_SIM__) -#ifndef CONFIG_XENO_OPT_DEBUG_NATIVE -#define CONFIG_XENO_OPT_DEBUG_NATIVE 0 -#endif - typedef xnticks_t RTIME; typedef xnsticks_t SRTIME; diff --git a/include/nucleus/assert.h b/include/nucleus/assert.h index 9cb88af..62bc329 100644 --- a/include/nucleus/assert.h +++ b/include/nucleus/assert.h @@ -38,4 +38,44 @@ xnpod_fatal("bug at %s:%d (%s)", __FILE__, __LINE__, (#cond)); \ } while(0) +#ifndef CONFIG_XENO_OPT_DEBUG_NATIVE +#define CONFIG_XENO_OPT_DEBUG_NATIVE 0 +#endif /* CONFIG_XENO_OPT_DEBUG_NATIVE */ +#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS +#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 +#endif /* CONFIG_XENO_OPT_DEBUG_NUCLEUS */ +#ifndef CONFIG_XENO_OPT_DEBUG_POSIX +#define CONFIG_XENO_OPT_DEBUG_POSIX 0 +#endif /* CONFIG_XENO_OPT_DEBUG_POSIX */ +#ifndef CONFIG_XENO_OPT_DEBUG_PSOS +#define CONFIG_XENO_OPT_DEBUG_PSOS 0 +#endif /* CONFIG_XENO_OPT_DEBUG_PSOS */ +#ifndef CONFIG_XENO_OPT_DEBUG_QUEUES +#define CONFIG_XENO_OPT_DEBUG_QUEUES 0 +#endif /* CONFIG_XENO_OPT_DEBUG_QUEUES */ +#ifndef CONFIG_XENO_OPT_DEBUG_REGISTRY +#define CONFIG_XENO_OPT_DEBUG_REGISTRY 0 +#endif /* CONFIG_XENO_OPT_DEBUG_REGISTRY */ +#ifndef CONFIG_XENO_OPT_DEBUG_RTDM +#define CONFIG_XENO_OPT_DEBUG_RTDM 0 +#endif /* CONFIG_XENO_OPT_DEBUG_RTDM */ +#ifndef CONFIG_XENO_OPT_DEBUG_RTDM_APPL +#define CONFIG_XENO_OPT_DEBUG_RTDM_APPL 0 +#endif /* CONFIG_XENO_OPT_DEBUG_RTDM_APPL */ +#ifndef CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX +#define CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX 0 +#endif /* CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX */ +#ifndef CONFIG_XENO_OPT_DEBUG_TIMERS +#define CONFIG_XENO_OPT_DEBUG_TIMERS 0 +#endif /* CONFIG_XENO_OPT_DEBUG_TIMERS */ +#ifndef CONFIG_XENO_OPT_DEBUG_UITRON +#define CONFIG_XENO_OPT_DEBUG_UITRON 0 +#endif /* CONFIG_XENO_OPT_DEBUG_UITRON */ +#ifndef CONFIG_XENO_OPT_DEBUG_VXWORKS +#define CONFIG_XENO_OPT_DEBUG_VXWORKS 0 +#endif /* CONFIG_XENO_OPT_DEBUG_VXWORKS */ +#ifndef CONFIG_XENO_OPT_DEBUG_XNLOCK +#define CONFIG_XENO_OPT_DEBUG_XNLOCK 0 +#endif /* CONFIG_XENO_OPT_DEBUG_XNLOCK */ + #endif /* !_XENO_NUCLEUS_ASSERT_H */ diff --git a/include/nucleus/bheap.h b/include/nucleus/bheap.h index 8b7798e..58ea93f 100644 --- a/include/nucleus/bheap.h +++ b/include/nucleus/bheap.h @@ -25,10 +25,6 @@ /* debug support */ #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_QUEUES -#define CONFIG_XENO_OPT_DEBUG_QUEUES 0 -#endif - /* Priority queue implementation, using a binary heap. */ typedef unsigned long long bheap_key_t; diff --git a/include/nucleus/heap.h b/include/nucleus/heap.h index 77fefa6..fecdb79 100644 --- a/include/nucleus/heap.h +++ b/include/nucleus/heap.h @@ -46,10 +46,6 @@ #if defined(__KERNEL__) || defined(__XENO_SIM__) -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - #define XNHEAP_PAGE_SIZE 512 /* A reasonable value for the xnheap page size */ #define XNHEAP_PAGE_MASK (~(XNHEAP_PAGE_SIZE-1)) #define XNHEAP_PAGE_ALIGN(addr) (((addr)+XNHEAP_PAGE_SIZE-1)&XNHEAP_PAGE_MASK) diff --git a/include/nucleus/pod.h b/include/nucleus/pod.h index e652a1e..8831fb5 100644 --- a/include/nucleus/pod.h +++ b/include/nucleus/pod.h @@ -276,14 +276,14 @@ static inline void xnpod_schedule(void) * context is active, or if we are caught in the middle of a * unlocked context switch. */ -#ifdef CONFIG_XENO_OPT_DEBUG_NUCLEUS +#if XENO_DEBUG(NUCLEUS) if (testbits(sched->status, XNKCOUT|XNINIRQ|XNSWLOCK)) return; -#else /* !CONFIG_XENO_OPT_DEBUG_NUCLEUS */ +#else /* !XENO_DEBUG(NUCLEUS) */ if (testbits(sched->status, XNKCOUT|XNINIRQ|XNSWLOCK|XNRESCHED) != XNRESCHED) return; -#endif /* !CONFIG_XENO_OPT_DEBUG_NUCLEUS */ +#endif /* !XENO_DEBUG(NUCLEUS) */ __xnpod_schedule(sched); } diff --git a/include/nucleus/queue.h b/include/nucleus/queue.h index e243f2f..15ec537 100644 --- a/include/nucleus/queue.h +++ b/include/nucleus/queue.h @@ -24,10 +24,6 @@ #include <nucleus/types.h> #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_QUEUES -#define CONFIG_XENO_OPT_DEBUG_QUEUES 0 -#endif - /* Basic element holder */ typedef struct xnholder { diff --git a/include/nucleus/sched-sporadic.h b/include/nucleus/sched-sporadic.h index ecebc55..dfeec76 100644 --- a/include/nucleus/sched-sporadic.h +++ b/include/nucleus/sched-sporadic.h @@ -29,10 +29,6 @@ #ifdef CONFIG_XENO_OPT_SCHED_SPORADIC -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - #include <nucleus/heap.h> extern struct xnsched_class xnsched_class_sporadic; diff --git a/include/nucleus/sched.h b/include/nucleus/sched.h index c96d65d..05dd4b1 100644 --- a/include/nucleus/sched.h +++ b/include/nucleus/sched.h @@ -36,10 +36,6 @@ #include <nucleus/sched-tp.h> #include <nucleus/sched-sporadic.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - /* Sched status flags */ #define XNKCOUT 0x80000000 /* Sched callout context */ #define XNHTICK 0x40000000 /* Host tick pending */ diff --git a/include/nucleus/synch.h b/include/nucleus/synch.h index bd9dbbc..d7edbc6 100644 --- a/include/nucleus/synch.h +++ b/include/nucleus/synch.h @@ -157,14 +157,14 @@ typedef struct xnsynch { extern "C" { #endif -#ifdef CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX +#if XENO_DEBUG(SYNCH_RELAX) void xnsynch_detect_relaxed_owner(struct xnsynch *synch, struct xnthread *sleeper); void xnsynch_detect_claimed_relax(struct xnthread *owner); -#else /* !CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX */ +#else /* !XENO_DEBUG(SYNCH_RELAX) */ static inline void xnsynch_detect_relaxed_owner(struct xnsynch *synch, struct xnthread *sleeper) @@ -175,7 +175,7 @@ static inline void xnsynch_detect_claimed_relax(struct xnthread *owner) { } -#endif /* !CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX */ +#endif /* !XENO_DEBUG(SYNCH_RELAX) */ void xnsynch_init(struct xnsynch *synch, xnflags_t flags, xnarch_atomic_t *fastlock); diff --git a/include/nucleus/timer.h b/include/nucleus/timer.h index ef05822..f2e0b19 100644 --- a/include/nucleus/timer.h +++ b/include/nucleus/timer.h @@ -28,10 +28,6 @@ #if defined(__KERNEL__) || defined(__XENO_SIM__) -#ifndef CONFIG_XENO_OPT_DEBUG_TIMERS -#define CONFIG_XENO_OPT_DEBUG_TIMERS 0 -#endif - #define XNTIMER_WHEELSIZE 64 #define XNTIMER_WHEELMASK (XNTIMER_WHEELSIZE - 1) diff --git a/include/psos+/ppd.h b/include/psos+/ppd.h index 09ea13f..82f7834 100644 --- a/include/psos+/ppd.h +++ b/include/psos+/ppd.h @@ -25,10 +25,6 @@ #include <nucleus/pod.h> #include <nucleus/ppd.h> -#ifndef CONFIG_XENO_OPT_DEBUG_PSOS -#define CONFIG_XENO_OPT_DEBUG_PSOS 0 -#endif - typedef struct psos_resource_holder { xnshadow_ppd_t ppd; diff --git a/include/rtdm/rtdm_driver.h b/include/rtdm/rtdm_driver.h index 0fc1496..0d11666 100644 --- a/include/rtdm/rtdm_driver.h +++ b/include/rtdm/rtdm_driver.h @@ -47,10 +47,6 @@ #include <asm-generic/xenomai/pci_ids.h> #endif /* CONFIG_PCI */ -#ifndef CONFIG_XENO_OPT_DEBUG_RTDM -#define CONFIG_XENO_OPT_DEBUG_RTDM 0 -#endif - struct rtdm_dev_context; typedef struct xnselector rtdm_selector_t; enum rtdm_selecttype; diff --git a/include/vxworks/ppd.h b/include/vxworks/ppd.h index 245c9b7..bae6fa1 100644 --- a/include/vxworks/ppd.h +++ b/include/vxworks/ppd.h @@ -25,10 +25,6 @@ #include <nucleus/pod.h> #include <nucleus/ppd.h> -#ifndef CONFIG_XENO_OPT_DEBUG_VXWORKS -#define CONFIG_XENO_OPT_DEBUG_VXWORKS 0 -#endif - typedef struct wind_resource_holder { xnshadow_ppd_t ppd; diff --git a/ksrc/nucleus/bufd.c b/ksrc/nucleus/bufd.c index 8dd655c..d7a7b00 100644 --- a/ksrc/nucleus/bufd.c +++ b/ksrc/nucleus/bufd.c @@ -146,10 +146,6 @@ #include <nucleus/bufd.h> #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - #ifdef CONFIG_XENO_OPT_PERVASIVE #include <asm/xenomai/syscall.h> diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c index 93713f2..3ffd548 100644 --- a/ksrc/nucleus/pod.c +++ b/ksrc/nucleus/pod.c @@ -46,10 +46,6 @@ #include <nucleus/select.h> #include <asm/xenomai/bits/pod.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - /* * NOTE: We need to initialize the globals; remember that this code * also runs over the simulator in user-space. @@ -2181,10 +2177,10 @@ void __xnpod_schedule(struct xnsched *sched) xnthread_current_priority(curr)); need_resched = __xnpod_test_resched(sched); -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS +#if !XENO_DEBUG(NUCLEUS) if (!need_resched) goto signal_unlock_and_exit; -#endif /* !CONFIG_XENO_OPT_DEBUG_NUCLEUS */ +#endif /* !XENO_DEBUG(NUCLEUS) */ zombie = xnthread_test_state(curr, XNZOMBIE); next = xnsched_pick_next(sched); diff --git a/ksrc/nucleus/registry.c b/ksrc/nucleus/registry.c index 9958dd3..8fb42fb 100644 --- a/ksrc/nucleus/registry.c +++ b/ksrc/nucleus/registry.c @@ -42,10 +42,6 @@ #include <nucleus/thread.h> #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_REGISTRY -#define CONFIG_XENO_OPT_DEBUG_REGISTRY 0 -#endif - static xnobject_t *registry_obj_slots; static xnqueue_t registry_obj_freeq; /* Free objects. */ diff --git a/ksrc/nucleus/sched.c b/ksrc/nucleus/sched.c index 4ec0013..0b737a3 100644 --- a/ksrc/nucleus/sched.c +++ b/ksrc/nucleus/sched.c @@ -529,10 +529,6 @@ void xnsched_migrate_passive(struct xnthread *thread, struct xnsched *sched) #ifdef CONFIG_XENO_OPT_SCALABLE_SCHED -#ifndef CONFIG_XENO_OPT_DEBUG_QUEUES -#define CONFIG_XENO_OPT_DEBUG_QUEUES 0 -#endif - void initmlq(struct xnsched_mlq *q, int loprio, int hiprio) { int prio; diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c index 539d952..872c37f 100644 --- a/ksrc/nucleus/shadow.c +++ b/ksrc/nucleus/shadow.c @@ -55,10 +55,6 @@ #include <asm/xenomai/syscall.h> #include <asm/xenomai/bits/shadow.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - static int xn_gid_arg = -1; module_param_named(xenomai_gid, xn_gid_arg, int, 0644); MODULE_PARM_DESC(xenomai_gid, "GID of the group with access to Xenomai services"); diff --git a/ksrc/nucleus/synch.c b/ksrc/nucleus/synch.c index 619929e..d57f7cd 100644 --- a/ksrc/nucleus/synch.c +++ b/ksrc/nucleus/synch.c @@ -36,10 +36,6 @@ #include <nucleus/thread.h> #include <nucleus/module.h> -#ifndef CONFIG_XENO_OPT_DEBUG_NUCLEUS -#define CONFIG_XENO_OPT_DEBUG_NUCLEUS 0 -#endif - #define w_bprio(t) xnsched_weighted_bprio(t) #define w_cprio(t) xnsched_weighted_cprio(t) @@ -981,7 +977,7 @@ void xnsynch_release_all_ownerships(struct xnthread *thread) } EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships); -#ifdef CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX +#if XENO_DEBUG(SYNCH_RELAX) /* * Detect when a thread is about to sleep on a synchronization @@ -1025,7 +1021,7 @@ void xnsynch_detect_claimed_relax(struct xnthread *owner) } } -#endif /* CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX */ +#endif /* XENO_DEBUG(SYNCH_RELAX) */ /*@}*/ diff --git a/ksrc/skins/posix/internal.h b/ksrc/skins/posix/internal.h index dcc71e9..46fa547 100644 --- a/ksrc/skins/posix/internal.h +++ b/ksrc/skins/posix/internal.h @@ -28,10 +28,6 @@ /* debug support */ #include <nucleus/assert.h> -#ifndef CONFIG_XENO_OPT_DEBUG_POSIX -#define CONFIG_XENO_OPT_DEBUG_POSIX 0 -#endif - #define PSE51_MAGIC(n) (0x8686##n##n) #define PSE51_ANY_MAGIC PSE51_MAGIC(00) #define PSE51_THREAD_MAGIC PSE51_MAGIC(01) diff --git a/ksrc/skins/rtdm/internal.h b/ksrc/skins/rtdm/internal.h index 69299f8..64e5b47 100644 --- a/ksrc/skins/rtdm/internal.h +++ b/ksrc/skins/rtdm/internal.h @@ -28,10 +28,6 @@ #include <nucleus/ppd.h> #include <rtdm/rtdm_driver.h> -#ifndef CONFIG_XENO_OPT_DEBUG_RTDM_APPL -#define CONFIG_XENO_OPT_DEBUG_RTDM_APPL 0 -#endif - #define RTDM_FD_MAX CONFIG_XENO_OPT_RTDM_FILDES #define DEF_DEVNAME_HASHTAB_SIZE 256 /* entries in name hash table */ diff --git a/ksrc/skins/uitron/ppd.h b/ksrc/skins/uitron/ppd.h index 07616ec..c851cb9 100644 --- a/ksrc/skins/uitron/ppd.h +++ b/ksrc/skins/uitron/ppd.h @@ -25,10 +25,6 @@ #include <nucleus/pod.h> #include <nucleus/ppd.h> -#ifndef CONFIG_XENO_OPT_DEBUG_UITRON -#define CONFIG_XENO_OPT_DEBUG_UITRON 0 -#endif - typedef struct ui_resource_holder { xnshadow_ppd_t ppd; diff --git a/src/skins/native/mutex.c b/src/skins/native/mutex.c index 98844ef..55d502f 100644 --- a/src/skins/native/mutex.c +++ b/src/skins/native/mutex.c @@ -103,7 +103,7 @@ static int rt_mutex_acquire_inner(RT_MUTEX *mutex, RTIME timeout, xntmode_t mode * mutex state consistent. * * We make no efforts to migrate or warn here. There is - * CONFIG_XENO_OPT_DEBUG_SYNCH_RELAX to catch such bugs. + * XENO_DEBUG(SYNCH_RELAX) to catch such bugs. */ if (mutex->lockcnt == UINT_MAX) return -EAGAIN; -- 1.5.6.5 -- Gilles. ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 13:58 [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs Gilles Chanteperdrix 2010-04-19 14:00 ` [Xenomai-core] [PATCH] debug: fix direct references to CONFIG_XENO_OPT_DEBUG_* Gilles Chanteperdrix 2010-04-19 14:05 ` [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs Gilles Chanteperdrix @ 2010-04-19 14:47 ` Jan Kiszka 2010-04-19 14:52 ` Gilles Chanteperdrix 2010-04-20 12:48 ` Gilles Chanteperdrix 2010-04-19 15:06 ` Philippe Gerum 3 siblings, 2 replies; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 14:47 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core 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. > - 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? Generally, I'm more a fan of decentralized management here (e.g. this avoids needless patch conflicts in central files). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 14:47 ` Jan Kiszka @ 2010-04-19 14:52 ` Gilles Chanteperdrix 2010-04-19 15:02 ` Jan Kiszka 2010-04-20 12:48 ` Gilles Chanteperdrix 1 sibling, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 14:52 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. > >> - 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 > > 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. -- Gilles. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 14:52 ` Gilles Chanteperdrix @ 2010-04-19 15:02 ` Jan Kiszka 2010-04-19 15:07 ` Gilles Chanteperdrix 0 siblings, 1 reply; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 15:02 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 15:02 ` Jan Kiszka @ 2010-04-19 15:07 ` Gilles Chanteperdrix 2010-04-19 15:33 ` Jan Kiszka 0 siblings, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 15:07 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 15:07 ` Gilles Chanteperdrix @ 2010-04-19 15:33 ` Jan Kiszka 2010-04-19 15:37 ` Gilles Chanteperdrix 0 siblings, 1 reply; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 15:33 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core 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. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 15:33 ` Jan Kiszka @ 2010-04-19 15:37 ` Gilles Chanteperdrix 2010-04-19 15:58 ` Jan Kiszka 0 siblings, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 15:37 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 15:37 ` Gilles Chanteperdrix @ 2010-04-19 15:58 ` Jan Kiszka 2010-04-19 16:10 ` Philippe Gerum 0 siblings, 1 reply; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 15:58 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 15:58 ` Jan Kiszka @ 2010-04-19 16:10 ` Philippe Gerum 2010-04-19 16:14 ` Jan Kiszka 0 siblings, 1 reply; 38+ messages in thread From: Philippe Gerum @ 2010-04-19 16:10 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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@domain.hid > https://mail.gna.org/listinfo/xenomai-core -- Philippe. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 16:10 ` Philippe Gerum @ 2010-04-19 16:14 ` Jan Kiszka 2010-04-19 16:25 ` Philippe Gerum 2010-04-19 16:27 ` Gilles Chanteperdrix 0 siblings, 2 replies; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 16:14 UTC (permalink / raw) To: Philippe Gerum; +Cc: xenomai-core 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. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 16:14 ` Jan Kiszka @ 2010-04-19 16:25 ` Philippe Gerum 2010-04-19 16:43 ` Philippe Gerum 2010-04-19 16:27 ` Gilles Chanteperdrix 1 sibling, 1 reply; 38+ messages in thread From: Philippe Gerum @ 2010-04-19 16:25 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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 decentralized is not important to you, then let's stop discussing about this, and find a centralized way that includes robustness, to avoid untested paths. IIRC, this was precisely the essence of Gilles's initial proposal. > > Jan > -- Philippe. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 16:25 ` Philippe Gerum @ 2010-04-19 16:43 ` Philippe Gerum 2010-04-19 17:22 ` Jan Kiszka 0 siblings, 1 reply; 38+ messages in thread From: Philippe Gerum @ 2010-04-19 16:43 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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 > decentralized is not important to you, then let's stop discussing about > this, and find a centralized way that includes robustness, to avoid > untested paths. IIRC, this was precisely the essence of Gilles's initial > proposal. By centralized, I don't mean into a single header file, I mean in the mainline code, possibly where it belongs if the symbol is purely local otherwise globally to avoid duplicates, using a common declaration pattern, with a builtin foolproof mechanism to trap misuses. We don't need Kconfig for that. > > > > > Jan > > > > -- Philippe. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 16:43 ` Philippe Gerum @ 2010-04-19 17:22 ` Jan Kiszka 2010-04-19 17:23 ` Philippe Gerum 2010-04-19 17:27 ` Philippe Gerum 0 siblings, 2 replies; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 17:22 UTC (permalink / raw) To: Philippe Gerum; +Cc: xenomai-core 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 17:22 ` Jan Kiszka @ 2010-04-19 17:23 ` Philippe Gerum 2010-04-19 17:27 ` Philippe Gerum 1 sibling, 0 replies; 38+ messages in thread From: Philippe Gerum @ 2010-04-19 17:23 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 17:22 ` Jan Kiszka 2010-04-19 17:23 ` Philippe Gerum @ 2010-04-19 17:27 ` Philippe Gerum 1 sibling, 0 replies; 38+ messages in thread From: Philippe Gerum @ 2010-04-19 17:27 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. > The point you have to keep in mind to understand why I would not merge anything like this is that adding a config knob to enable a particular debug subsystem, is fine, smart and makes a lot of sense. But having to tweak the config system to deal with obscure internal issues we may have for using those knobs is wrong. > Jan > -- Philippe. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 16:14 ` Jan Kiszka 2010-04-19 16:25 ` Philippe Gerum @ 2010-04-19 16:27 ` Gilles Chanteperdrix 2010-04-19 17:21 ` Jan Kiszka 1 sibling, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 16:27 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 16:27 ` Gilles Chanteperdrix @ 2010-04-19 17:21 ` Jan Kiszka 2010-04-19 17:26 ` Gilles Chanteperdrix 0 siblings, 1 reply; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 17:21 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core 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. 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). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 17:21 ` Jan Kiszka @ 2010-04-19 17:26 ` Gilles Chanteperdrix 2010-04-19 17:43 ` Jan Kiszka 0 siblings, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 17:26 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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) > > 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. -- Gilles. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 17:26 ` Gilles Chanteperdrix @ 2010-04-19 17:43 ` Jan Kiszka 2010-04-19 17:56 ` Gilles Chanteperdrix 2010-04-19 18:03 ` Gilles Chanteperdrix 0 siblings, 2 replies; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 17:43 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core [-- Attachment #1: Type: text/plain, Size: 3594 bytes --] 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). Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 17:43 ` Jan Kiszka @ 2010-04-19 17:56 ` Gilles Chanteperdrix 2010-04-19 18:03 ` Gilles Chanteperdrix 1 sibling, 0 replies; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 17:56 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 17:43 ` Jan Kiszka 2010-04-19 17:56 ` Gilles Chanteperdrix @ 2010-04-19 18:03 ` Gilles Chanteperdrix 2010-04-19 18:14 ` Gilles Chanteperdrix 1 sibling, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 18:03 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 18:03 ` Gilles Chanteperdrix @ 2010-04-19 18:14 ` Gilles Chanteperdrix 2010-04-19 18:18 ` Jan Kiszka 0 siblings, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 18:14 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 18:14 ` Gilles Chanteperdrix @ 2010-04-19 18:18 ` Jan Kiszka 2010-04-19 18:22 ` Gilles Chanteperdrix 0 siblings, 1 reply; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 18:18 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core [-- Attachment #1: Type: text/plain, Size: 5113 bytes --] Gilles Chanteperdrix wrote: > 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. > 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. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 18:18 ` Jan Kiszka @ 2010-04-19 18:22 ` Gilles Chanteperdrix 2010-04-19 18:38 ` Jan Kiszka 0 siblings, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 18:22 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> 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. >> > > 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. 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. -- Gilles. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 18:22 ` Gilles Chanteperdrix @ 2010-04-19 18:38 ` Jan Kiszka 2010-04-19 18:41 ` Gilles Chanteperdrix 0 siblings, 1 reply; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 18:38 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core [-- Attachment #1.1: Type: text/plain, Size: 1578 bytes --] 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 [-- Attachment #1.2: output.i --] [-- Type: text/plain, Size: 34554 bytes --] # 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 "/usr/include/bits/types.h" 1 3 4 # 28 "/usr/include/bits/types.h" 3 4 # 1 "/usr/include/bits/wordsize.h" 1 3 4 # 29 "/usr/include/bits/types.h" 2 3 4 typedef unsigned char __u_char; typedef unsigned short int __u_short; typedef unsigned int __u_int; typedef unsigned long int __u_long; typedef signed char __int8_t; typedef unsigned char __uint8_t; typedef signed short int __int16_t; typedef unsigned short int __uint16_t; typedef signed int __int32_t; typedef unsigned int __uint32_t; typedef signed long int __int64_t; typedef unsigned long int __uint64_t; typedef long int __quad_t; typedef unsigned long int __u_quad_t; # 131 "/usr/include/bits/types.h" 3 4 # 1 "/usr/include/bits/typesizes.h" 1 3 4 # 132 "/usr/include/bits/types.h" 2 3 4 typedef unsigned long int __dev_t; typedef unsigned int __uid_t; typedef unsigned int __gid_t; typedef unsigned long int __ino_t; typedef unsigned long int __ino64_t; typedef unsigned int __mode_t; typedef unsigned long int __nlink_t; typedef long int __off_t; typedef long int __off64_t; typedef int __pid_t; typedef struct { int __val[2]; } __fsid_t; typedef long int __clock_t; typedef unsigned long int __rlim_t; typedef unsigned long int __rlim64_t; typedef unsigned int __id_t; typedef long int __time_t; typedef unsigned int __useconds_t; typedef long int __suseconds_t; typedef int __daddr_t; typedef long int __swblk_t; typedef int __key_t; typedef int __clockid_t; typedef void * __timer_t; typedef long int __blksize_t; typedef long int __blkcnt_t; typedef long int __blkcnt64_t; typedef unsigned long int __fsblkcnt_t; typedef unsigned long int __fsblkcnt64_t; typedef unsigned long int __fsfilcnt_t; typedef unsigned long int __fsfilcnt64_t; typedef long int __ssize_t; typedef __off64_t __loff_t; typedef __quad_t *__qaddr_t; typedef char *__caddr_t; typedef long int __intptr_t; typedef unsigned int __socklen_t; # 32 "/usr/include/sys/types.h" 2 3 4 typedef __u_char u_char; typedef __u_short u_short; typedef __u_int u_int; typedef __u_long u_long; typedef __quad_t quad_t; typedef __u_quad_t u_quad_t; typedef __fsid_t fsid_t; typedef __loff_t loff_t; typedef __ino_t ino_t; # 62 "/usr/include/sys/types.h" 3 4 typedef __dev_t dev_t; typedef __gid_t gid_t; typedef __mode_t mode_t; typedef __nlink_t nlink_t; typedef __uid_t uid_t; typedef __off_t off_t; # 100 "/usr/include/sys/types.h" 3 4 typedef __pid_t pid_t; typedef __id_t id_t; typedef __ssize_t ssize_t; typedef __daddr_t daddr_t; typedef __caddr_t caddr_t; typedef __key_t key_t; # 133 "/usr/include/sys/types.h" 3 4 # 1 "/usr/include/time.h" 1 3 4 # 75 "/usr/include/time.h" 3 4 typedef __time_t time_t; # 93 "/usr/include/time.h" 3 4 typedef __clockid_t clockid_t; # 105 "/usr/include/time.h" 3 4 typedef __timer_t timer_t; # 134 "/usr/include/sys/types.h" 2 3 4 # 147 "/usr/include/sys/types.h" 3 4 # 1 "/usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h" 1 3 4 # 148 "/usr/include/sys/types.h" 2 3 4 typedef unsigned long int ulong; typedef unsigned short int ushort; typedef unsigned int uint; # 195 "/usr/include/sys/types.h" 3 4 typedef int int8_t __attribute__ ((__mode__ (__QI__))); typedef int int16_t __attribute__ ((__mode__ (__HI__))); typedef int int32_t __attribute__ ((__mode__ (__SI__))); typedef int int64_t __attribute__ ((__mode__ (__DI__))); typedef unsigned int u_int8_t __attribute__ ((__mode__ (__QI__))); typedef unsigned int u_int16_t __attribute__ ((__mode__ (__HI__))); typedef unsigned int u_int32_t __attribute__ ((__mode__ (__SI__))); typedef unsigned int u_int64_t __attribute__ ((__mode__ (__DI__))); typedef int register_t __attribute__ ((__mode__ (__word__))); # 217 "/usr/include/sys/types.h" 3 4 # 1 "/usr/include/endian.h" 1 3 4 # 37 "/usr/include/endian.h" 3 4 # 1 "/usr/include/bits/endian.h" 1 3 4 # 38 "/usr/include/endian.h" 2 3 4 # 61 "/usr/include/endian.h" 3 4 # 1 "/usr/include/bits/byteswap.h" 1 3 4 # 28 "/usr/include/bits/byteswap.h" 3 4 # 1 "/usr/include/bits/wordsize.h" 1 3 4 # 29 "/usr/include/bits/byteswap.h" 2 3 4 # 62 "/usr/include/endian.h" 2 3 4 # 218 "/usr/include/sys/types.h" 2 3 4 # 1 "/usr/include/sys/select.h" 1 3 4 # 31 "/usr/include/sys/select.h" 3 4 # 1 "/usr/include/bits/select.h" 1 3 4 # 32 "/usr/include/sys/select.h" 2 3 4 # 1 "/usr/include/bits/sigset.h" 1 3 4 # 24 "/usr/include/bits/sigset.h" 3 4 typedef int __sig_atomic_t; typedef struct { unsigned long int __val[(1024 / (8 * sizeof (unsigned long int)))]; } __sigset_t; # 35 "/usr/include/sys/select.h" 2 3 4 typedef __sigset_t sigset_t; # 1 "/usr/include/time.h" 1 3 4 # 121 "/usr/include/time.h" 3 4 struct timespec { __time_t tv_sec; long int tv_nsec; }; # 45 "/usr/include/sys/select.h" 2 3 4 # 1 "/usr/include/bits/time.h" 1 3 4 # 69 "/usr/include/bits/time.h" 3 4 struct timeval { __time_t tv_sec; __suseconds_t tv_usec; }; # 47 "/usr/include/sys/select.h" 2 3 4 typedef __suseconds_t suseconds_t; typedef long int __fd_mask; # 67 "/usr/include/sys/select.h" 3 4 typedef struct { __fd_mask __fds_bits[1024 / (8 * sizeof (__fd_mask))]; } fd_set; typedef __fd_mask fd_mask; # 99 "/usr/include/sys/select.h" 3 4 # 109 "/usr/include/sys/select.h" 3 4 extern int select (int __nfds, fd_set *__restrict __readfds, fd_set *__restrict __writefds, fd_set *__restrict __exceptfds, struct timeval *__restrict __timeout); # 121 "/usr/include/sys/select.h" 3 4 extern int pselect (int __nfds, fd_set *__restrict __readfds, fd_set *__restrict __writefds, fd_set *__restrict __exceptfds, const struct timespec *__restrict __timeout, const __sigset_t *__restrict __sigmask); # 221 "/usr/include/sys/types.h" 2 3 4 # 1 "/usr/include/sys/sysmacros.h" 1 3 4 # 30 "/usr/include/sys/sysmacros.h" 3 4 __extension__ extern unsigned int gnu_dev_major (unsigned long long int __dev) __attribute__ ((__nothrow__)); __extension__ extern unsigned int gnu_dev_minor (unsigned long long int __dev) __attribute__ ((__nothrow__)); __extension__ extern unsigned long long int gnu_dev_makedev (unsigned int __major, unsigned int __minor) __attribute__ ((__nothrow__)); # 224 "/usr/include/sys/types.h" 2 3 4 # 235 "/usr/include/sys/types.h" 3 4 typedef __blkcnt_t blkcnt_t; typedef __fsblkcnt_t fsblkcnt_t; typedef __fsfilcnt_t fsfilcnt_t; # 270 "/usr/include/sys/types.h" 3 4 # 1 "/usr/include/bits/pthreadtypes.h" 1 3 4 # 23 "/usr/include/bits/pthreadtypes.h" 3 4 # 1 "/usr/include/bits/wordsize.h" 1 3 4 # 24 "/usr/include/bits/pthreadtypes.h" 2 3 4 # 50 "/usr/include/bits/pthreadtypes.h" 3 4 typedef unsigned long int pthread_t; typedef union { char __size[56]; long int __align; } pthread_attr_t; typedef struct __pthread_internal_list { struct __pthread_internal_list *__prev; struct __pthread_internal_list *__next; } __pthread_list_t; # 76 "/usr/include/bits/pthreadtypes.h" 3 4 typedef union { struct __pthread_mutex_s { int __lock; unsigned int __count; int __owner; unsigned int __nusers; int __kind; int __spins; __pthread_list_t __list; # 101 "/usr/include/bits/pthreadtypes.h" 3 4 } __data; char __size[40]; long int __align; } pthread_mutex_t; typedef union { char __size[4]; int __align; } pthread_mutexattr_t; typedef union { struct { int __lock; unsigned int __futex; __extension__ unsigned long long int __total_seq; __extension__ unsigned long long int __wakeup_seq; __extension__ unsigned long long int __woken_seq; void *__mutex; unsigned int __nwaiters; unsigned int __broadcast_seq; } __data; char __size[48]; __extension__ long long int __align; } pthread_cond_t; typedef union { char __size[4]; int __align; } pthread_condattr_t; typedef unsigned int pthread_key_t; typedef int pthread_once_t; typedef union { struct { int __lock; unsigned int __nr_readers; unsigned int __readers_wakeup; unsigned int __writer_wakeup; unsigned int __nr_readers_queued; unsigned int __nr_writers_queued; int __writer; int __shared; unsigned long int __pad1; unsigned long int __pad2; unsigned int __flags; } __data; # 187 "/usr/include/bits/pthreadtypes.h" 3 4 char __size[56]; long int __align; } pthread_rwlock_t; typedef union { char __size[8]; long int __align; } pthread_rwlockattr_t; typedef volatile int pthread_spinlock_t; typedef union { char __size[32]; long int __align; } pthread_barrier_t; typedef union { char __size[4]; int __align; } pthread_barrierattr_t; # 271 "/usr/include/sys/types.h" 2 3 4 # 321 "/usr/include/stdlib.h" 2 3 4 extern long int random (void) __attribute__ ((__nothrow__)); extern void srandom (unsigned int __seed) __attribute__ ((__nothrow__)); extern char *initstate (unsigned int __seed, char *__statebuf, size_t __statelen) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (2))); extern char *setstate (char *__statebuf) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); struct random_data { int32_t *fptr; int32_t *rptr; int32_t *state; int rand_type; int rand_deg; int rand_sep; int32_t *end_ptr; }; extern int random_r (struct random_data *__restrict __buf, int32_t *__restrict __result) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern int srandom_r (unsigned int __seed, struct random_data *__buf) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (2))); extern int initstate_r (unsigned int __seed, char *__restrict __statebuf, size_t __statelen, struct random_data *__restrict __buf) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (2, 4))); extern int setstate_r (char *__restrict __statebuf, struct random_data *__restrict __buf) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern int rand (void) __attribute__ ((__nothrow__)); extern void srand (unsigned int __seed) __attribute__ ((__nothrow__)); extern int rand_r (unsigned int *__seed) __attribute__ ((__nothrow__)); extern double drand48 (void) __attribute__ ((__nothrow__)); extern double erand48 (unsigned short int __xsubi[3]) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); extern long int lrand48 (void) __attribute__ ((__nothrow__)); extern long int nrand48 (unsigned short int __xsubi[3]) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); extern long int mrand48 (void) __attribute__ ((__nothrow__)); extern long int jrand48 (unsigned short int __xsubi[3]) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); extern void srand48 (long int __seedval) __attribute__ ((__nothrow__)); extern unsigned short int *seed48 (unsigned short int __seed16v[3]) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); extern void lcong48 (unsigned short int __param[7]) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); struct drand48_data { unsigned short int __x[3]; unsigned short int __old_x[3]; unsigned short int __c; unsigned short int __init; unsigned long long int __a; }; extern int drand48_r (struct drand48_data *__restrict __buffer, double *__restrict __result) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern int erand48_r (unsigned short int __xsubi[3], struct drand48_data *__restrict __buffer, double *__restrict __result) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern int lrand48_r (struct drand48_data *__restrict __buffer, long int *__restrict __result) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern int nrand48_r (unsigned short int __xsubi[3], struct drand48_data *__restrict __buffer, long int *__restrict __result) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern int mrand48_r (struct drand48_data *__restrict __buffer, long int *__restrict __result) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern int jrand48_r (unsigned short int __xsubi[3], struct drand48_data *__restrict __buffer, long int *__restrict __result) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern int srand48_r (long int __seedval, struct drand48_data *__buffer) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (2))); extern int seed48_r (unsigned short int __seed16v[3], struct drand48_data *__buffer) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern int lcong48_r (unsigned short int __param[7], struct drand48_data *__buffer) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1, 2))); extern void *malloc (size_t __size) __attribute__ ((__nothrow__)) __attribute__ ((__malloc__)) ; extern void *calloc (size_t __nmemb, size_t __size) __attribute__ ((__nothrow__)) __attribute__ ((__malloc__)) ; extern void *realloc (void *__ptr, size_t __size) __attribute__ ((__nothrow__)) __attribute__ ((__warn_unused_result__)); extern void free (void *__ptr) __attribute__ ((__nothrow__)); extern void cfree (void *__ptr) __attribute__ ((__nothrow__)); # 1 "/usr/include/alloca.h" 1 3 4 # 25 "/usr/include/alloca.h" 3 4 # 1 "/usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h" 1 3 4 # 26 "/usr/include/alloca.h" 2 3 4 extern void *alloca (size_t __size) __attribute__ ((__nothrow__)); # 498 "/usr/include/stdlib.h" 2 3 4 extern void *valloc (size_t __size) __attribute__ ((__nothrow__)) __attribute__ ((__malloc__)) ; extern int posix_memalign (void **__memptr, size_t __alignment, size_t __size) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ; extern void abort (void) __attribute__ ((__nothrow__)) __attribute__ ((__noreturn__)); extern int atexit (void (*__func) (void)) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); extern int on_exit (void (*__func) (int __status, void *__arg), void *__arg) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); extern void exit (int __status) __attribute__ ((__nothrow__)) __attribute__ ((__noreturn__)); # 543 "/usr/include/stdlib.h" 3 4 extern char *getenv (__const char *__name) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ; extern char *__secure_getenv (__const char *__name) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ; extern int putenv (char *__string) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); extern int setenv (__const char *__name, __const char *__value, int __replace) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (2))); extern int unsetenv (__const char *__name) __attribute__ ((__nothrow__)); extern int clearenv (void) __attribute__ ((__nothrow__)); # 583 "/usr/include/stdlib.h" 3 4 extern char *mktemp (char *__template) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ; # 594 "/usr/include/stdlib.h" 3 4 extern int mkstemp (char *__template) __attribute__ ((__nonnull__ (1))) ; # 614 "/usr/include/stdlib.h" 3 4 extern char *mkdtemp (char *__template) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ; # 640 "/usr/include/stdlib.h" 3 4 extern int system (__const char *__command) ; # 662 "/usr/include/stdlib.h" 3 4 extern char *realpath (__const char *__restrict __name, char *__restrict __resolved) __attribute__ ((__nothrow__)) ; typedef int (*__compar_fn_t) (__const void *, __const void *); # 680 "/usr/include/stdlib.h" 3 4 extern void *bsearch (__const void *__key, __const void *__base, size_t __nmemb, size_t __size, __compar_fn_t __compar) __attribute__ ((__nonnull__ (1, 2, 5))) ; extern void qsort (void *__base, size_t __nmemb, size_t __size, __compar_fn_t __compar) __attribute__ ((__nonnull__ (1, 4))); # 699 "/usr/include/stdlib.h" 3 4 extern int abs (int __x) __attribute__ ((__nothrow__)) __attribute__ ((__const__)) ; extern long int labs (long int __x) __attribute__ ((__nothrow__)) __attribute__ ((__const__)) ; extern div_t div (int __numer, int __denom) __attribute__ ((__nothrow__)) __attribute__ ((__const__)) ; extern ldiv_t ldiv (long int __numer, long int __denom) __attribute__ ((__nothrow__)) __attribute__ ((__const__)) ; # 735 "/usr/include/stdlib.h" 3 4 extern char *ecvt (double __value, int __ndigit, int *__restrict __decpt, int *__restrict __sign) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3, 4))) ; extern char *fcvt (double __value, int __ndigit, int *__restrict __decpt, int *__restrict __sign) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3, 4))) ; extern char *gcvt (double __value, int __ndigit, char *__buf) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3))) ; extern char *qecvt (long double __value, int __ndigit, int *__restrict __decpt, int *__restrict __sign) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3, 4))) ; extern char *qfcvt (long double __value, int __ndigit, int *__restrict __decpt, int *__restrict __sign) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3, 4))) ; extern char *qgcvt (long double __value, int __ndigit, char *__buf) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3))) ; extern int ecvt_r (double __value, int __ndigit, int *__restrict __decpt, int *__restrict __sign, char *__restrict __buf, size_t __len) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3, 4, 5))); extern int fcvt_r (double __value, int __ndigit, int *__restrict __decpt, int *__restrict __sign, char *__restrict __buf, size_t __len) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3, 4, 5))); extern int qecvt_r (long double __value, int __ndigit, int *__restrict __decpt, int *__restrict __sign, char *__restrict __buf, size_t __len) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3, 4, 5))); extern int qfcvt_r (long double __value, int __ndigit, int *__restrict __decpt, int *__restrict __sign, char *__restrict __buf, size_t __len) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (3, 4, 5))); extern int mblen (__const char *__s, size_t __n) __attribute__ ((__nothrow__)) ; extern int mbtowc (wchar_t *__restrict __pwc, __const char *__restrict __s, size_t __n) __attribute__ ((__nothrow__)) ; extern int wctomb (char *__s, wchar_t __wchar) __attribute__ ((__nothrow__)) ; extern size_t mbstowcs (wchar_t *__restrict __pwcs, __const char *__restrict __s, size_t __n) __attribute__ ((__nothrow__)); extern size_t wcstombs (char *__restrict __s, __const wchar_t *__restrict __pwcs, size_t __n) __attribute__ ((__nothrow__)); extern int rpmatch (__const char *__response) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))) ; # 840 "/usr/include/stdlib.h" 3 4 extern int posix_openpt (int __oflag) ; # 875 "/usr/include/stdlib.h" 3 4 extern int getloadavg (double __loadavg[], int __nelem) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (1))); # 891 "/usr/include/stdlib.h" 3 4 # 4 "test-debug1.c" 2 # 1 "/usr/include/stdio.h" 1 3 4 # 30 "/usr/include/stdio.h" 3 4 # 1 "/usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h" 1 3 4 # 35 "/usr/include/stdio.h" 2 3 4 # 45 "/usr/include/stdio.h" 3 4 struct _IO_FILE; typedef struct _IO_FILE FILE; # 65 "/usr/include/stdio.h" 3 4 typedef struct _IO_FILE __FILE; # 75 "/usr/include/stdio.h" 3 4 # 1 "/usr/include/libio.h" 1 3 4 # 32 "/usr/include/libio.h" 3 4 # 1 "/usr/include/_G_config.h" 1 3 4 # 15 "/usr/include/_G_config.h" 3 4 # 1 "/usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h" 1 3 4 # 16 "/usr/include/_G_config.h" 2 3 4 # 1 "/usr/include/wchar.h" 1 3 4 # 78 "/usr/include/wchar.h" 3 4 typedef struct { int __count; union { unsigned int __wch; char __wchb[4]; } __value; } __mbstate_t; # 21 "/usr/include/_G_config.h" 2 3 4 typedef struct { __off_t __pos; __mbstate_t __state; } _G_fpos_t; typedef struct { __off64_t __pos; __mbstate_t __state; } _G_fpos64_t; # 53 "/usr/include/_G_config.h" 3 4 typedef int _G_int16_t __attribute__ ((__mode__ (__HI__))); typedef int _G_int32_t __attribute__ ((__mode__ (__SI__))); typedef unsigned int _G_uint16_t __attribute__ ((__mode__ (__HI__))); typedef unsigned int _G_uint32_t __attribute__ ((__mode__ (__SI__))); # 33 "/usr/include/libio.h" 2 3 4 # 53 "/usr/include/libio.h" 3 4 # 1 "/usr/lib64/gcc/x86_64-suse-linux/4.3/include/stdarg.h" 1 3 4 # 43 "/usr/lib64/gcc/x86_64-suse-linux/4.3/include/stdarg.h" 3 4 typedef __builtin_va_list __gnuc_va_list; # 54 "/usr/include/libio.h" 2 3 4 # 170 "/usr/include/libio.h" 3 4 struct _IO_jump_t; struct _IO_FILE; # 180 "/usr/include/libio.h" 3 4 typedef void _IO_lock_t; struct _IO_marker { struct _IO_marker *_next; struct _IO_FILE *_sbuf; int _pos; # 203 "/usr/include/libio.h" 3 4 }; enum __codecvt_result { __codecvt_ok, __codecvt_partial, __codecvt_error, __codecvt_noconv }; # 271 "/usr/include/libio.h" 3 4 struct _IO_FILE { int _flags; char* _IO_read_ptr; char* _IO_read_end; char* _IO_read_base; char* _IO_write_base; char* _IO_write_ptr; char* _IO_write_end; char* _IO_buf_base; char* _IO_buf_end; char *_IO_save_base; char *_IO_backup_base; char *_IO_save_end; struct _IO_marker *_markers; struct _IO_FILE *_chain; int _fileno; int _flags2; __off_t _old_offset; unsigned short _cur_column; signed char _vtable_offset; char _shortbuf[1]; _IO_lock_t *_lock; # 319 "/usr/include/libio.h" 3 4 __off64_t _offset; # 328 "/usr/include/libio.h" 3 4 void *__pad1; void *__pad2; void *__pad3; void *__pad4; size_t __pad5; int _mode; char _unused2[15 * sizeof (int) - 4 * sizeof (void *) - sizeof (size_t)]; }; typedef struct _IO_FILE _IO_FILE; struct _IO_FILE_plus; extern struct _IO_FILE_plus _IO_2_1_stdin_; extern struct _IO_FILE_plus _IO_2_1_stdout_; extern struct _IO_FILE_plus _IO_2_1_stderr_; # 364 "/usr/include/libio.h" 3 4 typedef __ssize_t __io_read_fn (void *__cookie, char *__buf, size_t __nbytes); typedef __ssize_t __io_write_fn (void *__cookie, __const char *__buf, size_t __n); typedef int __io_seek_fn (void *__cookie, __off64_t *__pos, int __w); typedef int __io_close_fn (void *__cookie); # 416 "/usr/include/libio.h" 3 4 extern int __underflow (_IO_FILE *); extern int __uflow (_IO_FILE *); extern int __overflow (_IO_FILE *, int); # 458 "/usr/include/libio.h" 3 4 extern int _IO_getc (_IO_FILE *__fp); extern int _IO_putc (int __c, _IO_FILE *__fp); extern int _IO_feof (_IO_FILE *__fp) __attribute__ ((__nothrow__)); extern int _IO_ferror (_IO_FILE *__fp) __attribute__ ((__nothrow__)); extern int _IO_peekc_locked (_IO_FILE *__fp); extern void _IO_flockfile (_IO_FILE *) __attribute__ ((__nothrow__)); extern void _IO_funlockfile (_IO_FILE *) __attribute__ ((__nothrow__)); extern int _IO_ftrylockfile (_IO_FILE *) __attribute__ ((__nothrow__)); # 488 "/usr/include/libio.h" 3 4 extern int _IO_vfscanf (_IO_FILE * __restrict, const char * __restrict, __gnuc_va_list, int *__restrict); extern int _IO_vfprintf (_IO_FILE *__restrict, const char *__restrict, __gnuc_va_list); extern __ssize_t _IO_padn (_IO_FILE *, int, __ssize_t); extern size_t _IO_sgetn (_IO_FILE *, void *, size_t); extern __off64_t _IO_seekoff (_IO_FILE *, __off64_t, int, int); extern __off64_t _IO_seekpos (_IO_FILE *, __off64_t, int); extern void _IO_free_backup_area (_IO_FILE *) __attribute__ ((__nothrow__)); # 76 "/usr/include/stdio.h" 2 3 4 # 89 "/usr/include/stdio.h" 3 4 typedef _G_fpos_t fpos_t; # 141 "/usr/include/stdio.h" 3 4 # 1 "/usr/include/bits/stdio_lim.h" 1 3 4 # 142 "/usr/include/stdio.h" 2 3 4 extern struct _IO_FILE *stdin; extern struct _IO_FILE *stdout; extern struct _IO_FILE *stderr; extern int remove (__const char *__filename) __attribute__ ((__nothrow__)); extern int rename (__const char *__old, __const char *__new) __attribute__ ((__nothrow__)); extern FILE *tmpfile (void) ; # 188 "/usr/include/stdio.h" 3 4 extern char *tmpnam (char *__s) __attribute__ ((__nothrow__)) ; extern char *tmpnam_r (char *__s) __attribute__ ((__nothrow__)) ; # 206 "/usr/include/stdio.h" 3 4 extern char *tempnam (__const char *__dir, __const char *__pfx) __attribute__ ((__nothrow__)) __attribute__ ((__malloc__)) ; extern int fclose (FILE *__stream); extern int fflush (FILE *__stream); # 231 "/usr/include/stdio.h" 3 4 extern int fflush_unlocked (FILE *__stream); # 245 "/usr/include/stdio.h" 3 4 extern FILE *fopen (__const char *__restrict __filename, __const char *__restrict __modes) ; extern FILE *freopen (__const char *__restrict __filename, __const char *__restrict __modes, FILE *__restrict __stream) ; # 274 "/usr/include/stdio.h" 3 4 # 285 "/usr/include/stdio.h" 3 4 extern FILE *fdopen (int __fd, __const char *__modes) __attribute__ ((__nothrow__)) ; # 306 "/usr/include/stdio.h" 3 4 extern void setbuf (FILE *__restrict __stream, char *__restrict __buf) __attribute__ ((__nothrow__)); extern int setvbuf (FILE *__restrict __stream, char *__restrict __buf, int __modes, size_t __n) __attribute__ ((__nothrow__)); extern void setbuffer (FILE *__restrict __stream, char *__restrict __buf, size_t __size) __attribute__ ((__nothrow__)); extern void setlinebuf (FILE *__stream) __attribute__ ((__nothrow__)); extern int fprintf (FILE *__restrict __stream, __const char *__restrict __format, ...); extern int printf (__const char *__restrict __format, ...); extern int sprintf (char *__restrict __s, __const char *__restrict __format, ...) __attribute__ ((__nothrow__)); extern int vfprintf (FILE *__restrict __s, __const char *__restrict __format, __gnuc_va_list __arg); extern int vprintf (__const char *__restrict __format, __gnuc_va_list __arg); extern int vsprintf (char *__restrict __s, __const char *__restrict __format, __gnuc_va_list __arg) __attribute__ ((__nothrow__)); extern int snprintf (char *__restrict __s, size_t __maxlen, __const char *__restrict __format, ...) __attribute__ ((__nothrow__)) __attribute__ ((__format__ (__printf__, 3, 4))); extern int vsnprintf (char *__restrict __s, size_t __maxlen, __const char *__restrict __format, __gnuc_va_list __arg) __attribute__ ((__nothrow__)) __attribute__ ((__format__ (__printf__, 3, 0))); # 400 "/usr/include/stdio.h" 3 4 extern int fscanf (FILE *__restrict __stream, __const char *__restrict __format, ...) ; extern int scanf (__const char *__restrict __format, ...) ; extern int sscanf (__const char *__restrict __s, __const char *__restrict __format, ...) __attribute__ ((__nothrow__)); # 443 "/usr/include/stdio.h" 3 4 # 506 "/usr/include/stdio.h" 3 4 extern int fgetc (FILE *__stream); extern int getc (FILE *__stream); extern int getchar (void); # 530 "/usr/include/stdio.h" 3 4 extern int getc_unlocked (FILE *__stream); extern int getchar_unlocked (void); # 541 "/usr/include/stdio.h" 3 4 extern int fgetc_unlocked (FILE *__stream); extern int fputc (int __c, FILE *__stream); extern int putc (int __c, FILE *__stream); extern int putchar (int __c); # 574 "/usr/include/stdio.h" 3 4 extern int fputc_unlocked (int __c, FILE *__stream); extern int putc_unlocked (int __c, FILE *__stream); extern int putchar_unlocked (int __c); extern int getw (FILE *__stream); extern int putw (int __w, FILE *__stream); extern char *fgets (char *__restrict __s, int __n, FILE *__restrict __stream) ; extern char *gets (char *__s) ; # 655 "/usr/include/stdio.h" 3 4 extern int fputs (__const char *__restrict __s, FILE *__restrict __stream); extern int puts (__const char *__s); extern int ungetc (int __c, FILE *__stream); extern size_t fread (void *__restrict __ptr, size_t __size, size_t __n, FILE *__restrict __stream) ; extern size_t fwrite (__const void *__restrict __ptr, size_t __size, size_t __n, FILE *__restrict __s) ; # 708 "/usr/include/stdio.h" 3 4 extern size_t fread_unlocked (void *__restrict __ptr, size_t __size, size_t __n, FILE *__restrict __stream) ; extern size_t fwrite_unlocked (__const void *__restrict __ptr, size_t __size, size_t __n, FILE *__restrict __stream) ; extern int fseek (FILE *__stream, long int __off, int __whence); extern long int ftell (FILE *__stream) ; extern void rewind (FILE *__stream); # 744 "/usr/include/stdio.h" 3 4 extern int fseeko (FILE *__stream, __off_t __off, int __whence); extern __off_t ftello (FILE *__stream) ; # 763 "/usr/include/stdio.h" 3 4 extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict __pos); extern int fsetpos (FILE *__stream, __const fpos_t *__pos); # 786 "/usr/include/stdio.h" 3 4 # 795 "/usr/include/stdio.h" 3 4 extern void clearerr (FILE *__stream) __attribute__ ((__nothrow__)); extern int feof (FILE *__stream) __attribute__ ((__nothrow__)) ; extern int ferror (FILE *__stream) __attribute__ ((__nothrow__)) ; extern void clearerr_unlocked (FILE *__stream) __attribute__ ((__nothrow__)); extern int feof_unlocked (FILE *__stream) __attribute__ ((__nothrow__)) ; extern int ferror_unlocked (FILE *__stream) __attribute__ ((__nothrow__)) ; extern void perror (__const char *__s); # 1 "/usr/include/bits/sys_errlist.h" 1 3 4 # 27 "/usr/include/bits/sys_errlist.h" 3 4 extern int sys_nerr; extern __const char *__const sys_errlist[]; # 825 "/usr/include/stdio.h" 2 3 4 extern int fileno (FILE *__stream) __attribute__ ((__nothrow__)) ; extern int fileno_unlocked (FILE *__stream) __attribute__ ((__nothrow__)) ; # 844 "/usr/include/stdio.h" 3 4 extern FILE *popen (__const char *__command, __const char *__modes) ; extern int pclose (FILE *__stream); extern char *ctermid (char *__s) __attribute__ ((__nothrow__)); # 884 "/usr/include/stdio.h" 3 4 extern void flockfile (FILE *__stream) __attribute__ ((__nothrow__)); extern int ftrylockfile (FILE *__stream) __attribute__ ((__nothrow__)) ; extern void funlockfile (FILE *__stream) __attribute__ ((__nothrow__)); # 914 "/usr/include/stdio.h" 3 4 # 5 "test-debug1.c" 2 # 25 "test-debug1.c" static const int XENO_OPT_DEBUG_NUCLEUS = 0; static const int CONFIG_XENO_OPT_DEBUG_NUCLEUS0 = 0; int main(void) { if ((y0 > XENO_OPT_DEBUG_NUCLEUS)) printf("Hello\n"); } [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 18:38 ` Jan Kiszka @ 2010-04-19 18:41 ` Gilles Chanteperdrix 2010-04-19 19:08 ` Jan Kiszka 0 siblings, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 18:41 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. -- Gilles. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 18:41 ` Gilles Chanteperdrix @ 2010-04-19 19:08 ` Jan Kiszka 2010-04-19 19:21 ` Gilles Chanteperdrix 0 siblings, 1 reply; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 19:08 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core [-- Attachment #1: Type: text/plain, Size: 2752 bytes --] 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. But as it still requires explicit declaration, I'm more in favor of diff --git a/scripts/prepare-kernel.sh b/scripts/prepare-kernel.sh index 24b1f17..d8038e0 100755 --- a/scripts/prepare-kernel.sh +++ b/scripts/prepare-kernel.sh @@ -584,6 +584,17 @@ for d in include/* ; do fi done +kconfigs=`find $xenomai_root/ksrc -name Kconfig` +debug_defines=$linux_tree/include/xenomai/nucleus/debug_defines.h +rm -f $debug_defines +for debugopt in `grep XENO_OPT_DEBUG_ $kconfigs | cut -d' ' -f2`; do + cat >>$debug_defines <<EOF +#ifndef CONFIG_$debugopt +#define CONFIG_$debugopt 0 +#endif +EOF +done + if test "x$output_patch" != "x"; then if test x$verbose = x1; then echo 'Generating patch.' (+ some magic for the simulator - if that thing still builds) Please let me know what precisely you dislike in this approach. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 19:08 ` Jan Kiszka @ 2010-04-19 19:21 ` Gilles Chanteperdrix 2010-04-19 19:37 ` Jan Kiszka 0 siblings, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 19:21 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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 case the kernel is never compiled without optimization, some other things break in that case. > But as it still requires explicit declaration, I'm more in favor of > (...) > > 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. And if you run it for every compilation, it wil slow the compilation down. And you will have to add some tricks to avoid touching the generated file if its contents did not change to avoid a full re-compilation. And if you do all that you will end-up at best with 20 lines of shell. The current candidate is 3 lines of C macro + 1 line per debug symbol. I do not like dynamic sources generation. Especially when we have a working solution based on C pre-processor macros. -- Gilles. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 19:21 ` Gilles Chanteperdrix @ 2010-04-19 19:37 ` Jan Kiszka 2010-04-19 19:47 ` Gilles Chanteperdrix 2010-04-19 19:55 ` Gilles Chanteperdrix 0 siblings, 2 replies; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 19:37 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core [-- Attachment #1: Type: text/plain, Size: 3474 bytes --] 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. > case the kernel is never compiled without optimization, some other > things break in that case. > >> But as it still requires explicit declaration, I'm more in favor of >> (...) >> >> 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. > And if you run it for every compilation, it wil slow the compilation down. > And you will have to add some tricks to avoid touching the generated > file if its contents did not change to avoid a full re-compilation. > And if you do all that you will end-up at best with 20 lines of shell. > > The current candidate is 3 lines of C macro + 1 line per debug symbol. The changes to assert.h do not matter that much. More important is that you do not need to bother about adding the declare somewhere. > > 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. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 19:37 ` Jan Kiszka @ 2010-04-19 19:47 ` Gilles Chanteperdrix 2010-04-19 20:08 ` Jan Kiszka 2010-04-19 19:55 ` Gilles Chanteperdrix 1 sibling, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 19:47 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 19:47 ` Gilles Chanteperdrix @ 2010-04-19 20:08 ` Jan Kiszka 2010-04-19 20:33 ` Gilles Chanteperdrix 0 siblings, 1 reply; 38+ messages in thread From: Jan Kiszka @ 2010-04-19 20:08 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core [-- Attachment #1: Type: text/plain, Size: 3791 bytes --] Gilles Chanteperdrix wrote: > 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. Right from that POV. XENO_DEBUG(foo) itself remains preprocessor stuff, ie. never includes potentially non-static expressions. But this part is academic if -O0 fails to build the kernel. > > 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. At worst, you run into a build error and repeat the step. Otherwise, the next installation will fix it silently. It's much like the missing DECLARE_ASSERT_SYMBOL detection and resolution, IMO even more convenient. > >>> 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. > But not #if XENO_DEBUG(DOES_NOT_EXIST) Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 20:08 ` Jan Kiszka @ 2010-04-19 20:33 ` Gilles Chanteperdrix 0 siblings, 0 replies; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 20:33 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core Jan Kiszka wrote: >>>>> 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. > > At worst, you run into a build error and repeat the step. Otherwise, the > next installation will fix it silently. It's much like the missing > DECLARE_ASSERT_SYMBOL detection and resolution, IMO even more convenient. I do not see in what way it is more convenient. I would now have to run prepare-kernel.sh in a case when I did not have to run it. Adding DECLARE_ASSERT_SYMBOL to one .c file just triggers the recompilation of the file, and relinking of the kernel. > >>>> 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. >> > > But not > > #if XENO_DEBUG(DOES_NOT_EXIST) This can be made during the build-test and thus avoided during a normal compilation. I am really, strongly opposed to dynamic code generation. Especially which greps the whole source tree, this is a waste of cpu power. Most shell scripts are wrong when first written and do not handle errors correctly. I already see two or three pitfalls in the script you proposed. Globally, this is really overkill and risky. The alternative, a 3 lines C macro, has the inconvenient of requiring that we declare the symbol, but after all the original solution had the same, and nobody complained. We do not add debug symbols that often. What is important however, is that we avoid the current mess which happened in pod.h and pod.c because they used #ifdef CONFIG_XENO_OPT_DEBUG_NUCLEUS, and this macro solution allows to detect such issues.. We can add the missing features of this solution in the build-test script. This way there is 0 impact on the compilation for end-users. And since we do run the build-test script, we are sure not to miss any error. And if the scripts erases xenomai sources directory because of flaky error checking, it will only erase my xenomai sources. -- Gilles. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 19:37 ` Jan Kiszka 2010-04-19 19:47 ` Gilles Chanteperdrix @ 2010-04-19 19:55 ` Gilles Chanteperdrix 1 sibling, 0 replies; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-19 19:55 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core Jan Kiszka wrote: > My prepare-kernel approach also detects stall debug stuff: The uitron > use of XENO_DEBUG is not backed by any config option. This can be checked when we do the build-test, this will be much more economic. -- Gilles. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 14:47 ` Jan Kiszka 2010-04-19 14:52 ` Gilles Chanteperdrix @ 2010-04-20 12:48 ` Gilles Chanteperdrix 2010-04-20 14:02 ` Gilles Chanteperdrix 1 sibling, 1 reply; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-20 12:48 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core 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. > >> - 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? > > Generally, I'm more a fan of decentralized management here (e.g. this > avoids needless patch conflicts in central files). My latest approach was not really working (it was working, but generated warning when compiled with -Wundef, and the kernel is compiled with -Wundef). So, let us stop the madness. I have merged the following solution: - fixed the direct references to CONFIG_XENO_OPT_DEBUG_FOO and replaced them with XENO_DEBUG(FOO) - moved the duplicate CONFIG_XENO_OPT_DEBUG_FOO initializations which did not fit to a particular file to assert.h (actually only QUEUES and NUCLEUS), let the other initializations live in peace where they were. This is complemented by a check_script whose result on the previous xenomai version you can see at: http://sisyphus.hd.free.fr/~gilles/bx/distcheck/xenomai/log.html -- Gilles. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-20 12:48 ` Gilles Chanteperdrix @ 2010-04-20 14:02 ` Gilles Chanteperdrix 0 siblings, 0 replies; 38+ messages in thread From: Gilles Chanteperdrix @ 2010-04-20 14:02 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core Gilles Chanteperdrix wrote: > This is complemented by a check_script whose result on the previous > xenomai version you can see at: > http://sisyphus.hd.free.fr/~gilles/bx/distcheck/xenomai/log.html Moved it to: http://sisyphus.hd.free.fr/~gilles/pastebin/wcZMMi to be able to run the build-test on master. -- Gilles. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs. 2010-04-19 13:58 [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs Gilles Chanteperdrix ` (2 preceding siblings ...) 2010-04-19 14:47 ` Jan Kiszka @ 2010-04-19 15:06 ` Philippe Gerum 3 siblings, 0 replies; 38+ messages in thread From: Philippe Gerum @ 2010-04-19 15:06 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2010-04-20 14:02 UTC | newest] Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-04-19 13:58 [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs Gilles Chanteperdrix 2010-04-19 14:00 ` [Xenomai-core] [PATCH] debug: fix direct references to CONFIG_XENO_OPT_DEBUG_* Gilles Chanteperdrix 2010-04-19 14:05 ` [Xenomai-core] [RFC] fix XENO_OPT_DEBUG bugs Gilles Chanteperdrix 2010-04-19 14:47 ` Jan Kiszka 2010-04-19 14:52 ` Gilles Chanteperdrix 2010-04-19 15:02 ` Jan Kiszka 2010-04-19 15:07 ` Gilles Chanteperdrix 2010-04-19 15:33 ` Jan Kiszka 2010-04-19 15:37 ` Gilles Chanteperdrix 2010-04-19 15:58 ` Jan Kiszka 2010-04-19 16:10 ` Philippe Gerum 2010-04-19 16:14 ` Jan Kiszka 2010-04-19 16:25 ` Philippe Gerum 2010-04-19 16:43 ` Philippe Gerum 2010-04-19 17:22 ` Jan Kiszka 2010-04-19 17:23 ` Philippe Gerum 2010-04-19 17:27 ` Philippe Gerum 2010-04-19 16:27 ` Gilles Chanteperdrix 2010-04-19 17:21 ` Jan Kiszka 2010-04-19 17:26 ` Gilles Chanteperdrix 2010-04-19 17:43 ` Jan Kiszka 2010-04-19 17:56 ` Gilles Chanteperdrix 2010-04-19 18:03 ` Gilles Chanteperdrix 2010-04-19 18:14 ` Gilles Chanteperdrix 2010-04-19 18:18 ` Jan Kiszka 2010-04-19 18:22 ` Gilles Chanteperdrix 2010-04-19 18:38 ` Jan Kiszka 2010-04-19 18:41 ` Gilles Chanteperdrix 2010-04-19 19:08 ` Jan Kiszka 2010-04-19 19:21 ` Gilles Chanteperdrix 2010-04-19 19:37 ` Jan Kiszka 2010-04-19 19:47 ` Gilles Chanteperdrix 2010-04-19 20:08 ` Jan Kiszka 2010-04-19 20:33 ` Gilles Chanteperdrix 2010-04-19 19:55 ` Gilles Chanteperdrix 2010-04-20 12:48 ` Gilles Chanteperdrix 2010-04-20 14:02 ` Gilles Chanteperdrix 2010-04-19 15:06 ` Philippe Gerum
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.