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

[Sebastian Huber <sebastian.huber@embedded-brains.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.