Re: [PATCH 2/3, newlib] Only define static locks in multithreaded mode

[Thomas Preudhomme <thomas.preudhomme@foss.arm.com>]

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