Re: [PATCH] Proper locking for getchar() and putchar()

[Sebastian Huber <sebastian dot huber at embedded-brains dot de>]

On 07/08/17 11:49, Corinna Vinschen wrote:

> Hi Sebastian,
>
> On Aug  7 08:33, Sebastian Huber wrote:
> > Add internal inline functions _getchar_unlocked() and
> > _putchar_unlocked() if __CUSTOM_FILE_IO__ is not defined.  These
> > functions get _REENT only once.  Use them for getchar_unlocked() and
> > putchar_unlocked().  Define getchar() and putchar() to these unlocked
> > internal functions if __SINGLE_THREAD__ is defined, otherwise use the
> > external functions to use proper locking of the FILE object.
> >
> > Assumes that __SINGLE_THREAD__ is not defined if __CYGWIN__ is defined.
> Yeah :)  But, even then, __SINGLE_THREAD__ is __SINGLE_THREAD__.
>
> > Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
> > ---
> >   newlib/libc/include/stdio.h | 39 ++++++++++++++++++++++++++++++++-------
> >   1 file changed, 32 insertions(+), 7 deletions(-)
> >
> > diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h
> > index 1c32423d3..5d8cb1092 100644
> > --- a/newlib/libc/include/stdio.h
> > +++ b/newlib/libc/include/stdio.h
> > @@ -735,14 +735,37 @@ _ELIDABLE_INLINE int __sputc_r(struct _reent *_ptr, int _c, FILE *_p) {
> >   #define	fileno(p)	__sfileno(p)
> >   #endif
> >   
> > -#ifndef __CYGWIN__
> > -#ifndef lint
> > -#define	getc(fp)	__sgetc_r(_REENT, fp)
> > -#define putc(x, fp)	__sputc_r(_REENT, x, fp)
> > -#endif /* lint */
> > -#endif /* __CYGWIN__ */
> > +static __inline int
> > +_getchar_unlocked(void)
> > +{
> > +	struct _reent *_ptr;
> > +
> > +	_ptr = _REENT;
> > +	return (__sgetc_r(_ptr, _stdin_r(_ptr)));
> > +}
> > +
> > +static __inline int
> > +_putchar_unlocked(int _c)
> > +{
> > +	struct _reent *_ptr;
> > +
> > +	_ptr = _REENT;
> > +	return (__sputc_r(_ptr, _c, _stdout_r(_ptr)));
> > +}
> > +
> > +#ifdef __SINGLE_THREAD__
> > +#define	getc(_p)	__sgetc_r(_REENT, _p)
> > +#define	putc(_c, _p)	__sputc_r(_REENT, _c, _p)
> > +#define	getchar()	_getchar_unlocked()
> > +#define	putchar(_c)	_putchar_unlocked(_c)
> > +#endif /* __SINGLE_THREAD__ */
> >   #endif /* __cplusplus */
> That looks good, but I wonder, wouldn't it make sense to replace the
> inline _getchar_unlocked/_putchar_unlocked with inline
> _getc_unlocked/_puts_unlocked?
>
> I'm asking because that would allow to #define all four,
> getc/putc/getchar/putchar, so that the parameters are only evaluated
> once.
>
> What do you think?

For getc()/putc() you have the FILE object already and only need _REENT.

For getchar()/putchar() you have nothing and need the _REENT plus 
stdin/stdout.

I don't know how you would unify/simplify this further?

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.