On 02/02/2017 09:32 AM, Thomas Preudhomme wrote:
On 01/02/17 08:48, Freddie Chopin wrote:
On Tue, 2017-01-31 at 17:10 +0000, Thomas Preudhomme wrote:
It also makes sure locking macros in lock.h are noop in single thread
mode.
Doesn't it make all the guards in the code superfluous? If the lock
initialization macros are defined to be ";" and lock functions are
defined to be "(void)0", then the guards in the code actually guard
nothing, as things like:
#if !defined(__SINGLE_THREAD__) && defined(HAVE_DD_LOCK)
__LOCK_INIT(static, __dd_hash_mutex);
#endif
#ifndef __SINGLE_THREAD__
__lock_acquire(__dd_hash_mutex);
#endif
will then be equivalent to:
#if !defined(__SINGLE_THREAD__) && defined(HAVE_DD_LOCK)
;
#endif
#ifndef __SINGLE_THREAD__
(void)0;
#endif
Yes it does and I started to remove them but noticed that removing the guard
made code more inconsistent. The example you give is a nice one because then
we would still have a condition with HAVE_DD_LOCK which might induce people to
think that's the only case where this line does nothing.
In some other case __SINGLE_THREAD__ guards more than just the lock. For
instance in newlib/libc/stdio/findfp.c __SINGLE_THREAD__ is used to guard
whether some function are defined. This would need to be kept so the only
change that could be done is take the lock out of the __SINGLE_THREAD__
guarded block which would look weird again.
I then wondered whether making the code noop in lock.h was the right approach
but decided that provided more safety that the code would not be compiled in
in case of missing guard (as you discovered for some of the locks). It also
goes with what is currently being done when targeting bare-metal since the
lock routines themselves are noop in that case. Only the declarations are not.
The one thing that could be done is to make the declaration noop in both
cases, thus reducing the complexity.
Any thoughts on that?
Best regards,
Thomas
Yes, a thought on the general approach. At this moment in time the locks define
to being degenerate so that the guards are superfluous. However, the guards
make it very clear on reading the "application" code that the locks are not used
in those cases, without needing to chase down what happens with the underlying
functions. In addition, the locks becoming degenerate in the guarded cases
could possibly change in the future. So the thought is that while the guards
could be removed, don't. This not only keeps the main code more clear in
intent, it saves the time to take them out, the time to check it, and could also
possibly save the time putting them back later on if the present degeneration
method were to be altered.