Re: Query concerning use of flockfile/funlockfile in stdio FILE relatedfunctions

[Antony KING <antony dot king at st dot com>]

Jeff Johnston wrote:
> Antony KING wrote:
>
> > Hello,
> >
> > I have a query concerning the use of the flockfile/funlockfile 
> > functions  in the stdio sub-component of newlib. It relates to a 
> > requirement we have of being able to call the sprintf/sscanf class of 
> > functions from interrupt handlers in our RTOS.
> >
> > One of the restrictions of interrupt handlers in our RTOS is that they 
> > cannot block on synchronisation objects which is unfortunate since the 
> > sprintf/sscanf class of objects implicitly use the locking API defined 
> > in sys/lock.h via flockfile/funlockfile (I have implemented the 
> > locking API using the synchronisation objects available in our RTOS).
>
> Anthony, in the sprintf/sscanf case, as you have noted, the lock object 
> is only known to the sprintf or sscanf functions (they are part of local 
> FILE objects).  Thus, there should never be a blocking situation which 
> technically meets the "letter of the law" stated above.  Is the 
> restriction more strict than you have noted?

No, except :-), that when the interrupt handlers are running we
deliberately set _REENT to NULL to simplify matters.

> I have noticed that the local FILE lock objects are not being 
> initialized which is an error :(.

Yes, so did I, and in my local development version of newlib I added the
initialisation and closure in order for sprintf et al to work when
called from task context. However, your suggestion of using __SSTR to
check whether locking should be applied or not means that the lock
object can be left uninitialised with the proviso that __SSTR is added
to the local FILE structures created by the sscanf/vsscanf functions as
you note below.

> > This restriction is not a problem when using FILE objects created via 
> > fopen et al since they normally refer to resources which require the 
> > proper thread protection afforded by the locking API. However for the 
> > sprintf/sscanf class of functions using the locking API is unnecessary 
> > since the FILE objects created by these functions for use by the 
> > "real" functions _vfprintf_r/__svfscanf_r should be inherently thread 
> > safe (as they are allocated off the stack).
> >
> > It should therefore be possible to dynamically detect this scenario 
> > and not use the locking API and hence allow me to use the 
> > sprintf/sscanf functions from my interrupt handlers.
> >
> > Ideally I would like to add a flag to the _flags field of the FILE 
> > structure to indicate whether a lock object is present or not but 
> > unfortunately there are no free bits left. Another possibility would 
> > have been to use the fact that the sprintf/sscanf fucntions set the 
> > _file field of the FILE structure to -1 but thus is not reliable since 
> > it can take the value of -1 in other instances.
> >
> > One thought is to extend the locking API to to include "not 
> > initialised" initialiser APIs whose purpose is to allow the locking 
> > API acquire and release methods detect this state and ignore the 
> > lock/unlock requests. Its not very nice but would work without 
> > changing the layout of the FILE structure (by increasing the size of 
> > the _flags field from 16 bits to 32 bits). Also I am uncertain how 
> > this change to the locking API would impact other implementations 
> > (e.g. linux/cygwin).
>
> The locking API should not be changed.  It is easy enough to check for 
> the __SSTR flag to avoid locking.  This flag is set for all the string 
> printf operations but isn't currently used for the sscanf set.
>
> -- Jeff J.

Given the existence of __SSTR I agree with your sentiment (extending the
locking API was making the implementation a little ugly) and I am now
using this as a mechanism to disable locking in the sprintf/sscanf set
of functions. One other minor change I am also making is a result of
_REENT being set to NULL when running an interrupt handler; the
definition of the macro CHECK_INIT has been modified to check that
_REENT is not NULL before checking the initialisation of _REENT.

I am in the process of making the relevant changes to newlib at present
all seems fine. I will submit this as a patch against the my local
snapshot of the CVS repository once complete.

Cheers,

Antony.

--
-----------------------------------------------------------------
Antony King                        |
STMicroelectronics (R&D) Ltd.      |
Bristol, BS32 4SQ. United Kingdom. |