Re: [PATCH, newlib] Allow locking routine to be retargeted

[Thomas Preudhomme <thomas dot preudhomme at foss dot arm dot com>]

On 24/01/17 09:58, Freddie Chopin wrote:
> On Fri, 2017-01-20 at 13:37 +0000, Thomas Preudhomme wrote:
> > Have you had time to work on the last version of the patch?
>
> OK, I finally got down to it.
>
> The toolchain builds, but this is obvious.
>
> Some of the locks are not always needed:
>
> 1. dd_hash_lock from posix/telldir.c is defined only if HAVE_DD_LOCK is
> defined. It seems that only linux and phoenix define that macro, but it
> would be OK to have this lock conditional.
>
> 2. __sfp_lock and __sinit_lock from stdio/findfp.c, __tz_lock_object
> from time/tzlock.c, __malloc_lock_object from stdlib/mlock.c and
> __env_lock_object from stdlib/envlock.c are defined only when
> __SINGLE_THREAD__ is _NOT_ defined.

> However __atexit_lock in
> stdlib/__call_atexit.c is defined in case of __SINGLE_THREAD__
> configuration, but it is not used, as all uses are conditional anyway.

I think it's fragile to rely on how it's used. Someone could add a new use and 
will not think of changing the definition of the __lock_ counterpart unless they 
are testing the retargetable lock feature.

> On the other hand dd_hash_lock and atexit_mutex are not protected with
> that macro. Maybe all locks should be conditional on this macro?

That sounds like a reasonable thing to do. Why would one need a lock in a single 
threaded environment? In that case I think the __SINGLE_THREAD__ guard should be 
right in lock.h which would clean up the code nicely.

I'll look into doing that.

Best regards,

Thomas