This is the mail archive of the
newlib@sources.redhat.com
mailing list for the newlib project.
Re: [PATCH] Fix problem with file locking used before initialised
- From: Antony KING <antony dot king at st dot com>
- To: Jeff Johnston <jjohnstn at redhat dot com>
- Cc: newlib at sources dot redhat dot com
- Date: Tue, 08 Feb 2005 18:46:58 +0000
- Subject: Re: [PATCH] Fix problem with file locking used before initialised
- Organization: STMicroelectronics (R&D) Ltd
- References: <41F53DD1.3070506@st.com> <41FFD4DA.3090305@st.com> <42081772.5050405@redhat.com>
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.
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).
Cheers,
Antony.
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.
Thanks,
-- Jeff J.