[PATCH] Fix problem with file locking used before initialised
Tue Feb 8 19:39:00 GMT 2005
Antony KING wrote:
> I have had a bit of a think on this and I don't think the addition of
> this lock to __sinit is required since __sinit is called with a task
> specific reent structure and does not reference any global state (other
> than that in the reent structure).
> __sinit should therefore be inherently thread safe without the addition
> of locks unless your application/RTOS is using this API in such a way
> that it is possible for one task to initialise the stdio data of another
> task's reent structure (and therefore have a potential race condition).
> However I believe this scenario is not the model expected, nor
> supported, by newlib (correct me if I am wrong in my understanding :-);
> the expected model, I believe, is that a reent structure should only be
> changed by its owner task.
You're right, a thinko on my part; I was also thinking of an enhancement I have
been contemplating which will allow sharing of the std streams via _GLOBAL_REENT
and having an alternate version of CHECK_INIT that ignores the input reentrant
structure and passes _GLOBAL_REENT to _sinit.
> The only place in newlib where this seems to happen is __sfp (also in
> findfp.c) where it checks the stdio initialisation of the global reent
> structure (_GLOBAL_REENT). However this is safe since __sfp is already
> taking a lock before the initialisation.
> Also, I note that I missed libgloss/arm/syscalls.c when I made the
> changes to newlib/libc/sys/arm/syscalls.c in my recent patch and
> therefore you may want to apply the same changes to this file (the files
> should be identical).
Actually, I had done this but forgot to check it in. Patch checked in.
-- Jeff J.
> Jeff Johnston wrote:
>> Patch applied. I noticed a flaw in that __sinit was unlocked so you
>> could conceivably have two threads that called it in a race
>> condition. I added a lock for it and added a check once you acquire
>> the lock that __sdidinit wasn't already set.
>> -- Jeff J.
More information about the Newlib