* [PATCH] Proper locking for getchar() and putchar()
@ 2017-08-07 6:33 Sebastian Huber
2017-08-07 9:50 ` Corinna Vinschen
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Huber @ 2017-08-07 6:33 UTC (permalink / raw)
To: newlib
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.
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 */
+#if __MISC_VISIBLE || __POSIX_VISIBLE
+#define getchar_unlocked() _getchar_unlocked()
+#define putchar_unlocked(_c) _putchar_unlocked(_c)
+#endif
+
#if __MISC_VISIBLE
/* fast always-buffered version, true iff error */
#define fast_putc(x,p) (--(p)->_w < 0 ? \
@@ -756,7 +779,7 @@ _ELIDABLE_INLINE int __sputc_r(struct _reent *_ptr, int _c, FILE *_p) {
#define L_ctermid 16
#endif
-#endif /* !__CUSTOM_FILE_IO__ */
+#else /* __CUSTOM_FILE_IO__ */
#define getchar() getc(stdin)
#define putchar(x) putc(x, stdout)
@@ -766,6 +789,8 @@ _ELIDABLE_INLINE int __sputc_r(struct _reent *_ptr, int _c, FILE *_p) {
#define putchar_unlocked(x) putc_unlocked(x, stdout)
#endif
+#endif /* !__CUSTOM_FILE_IO__ */
+
_END_STD_C
#endif /* _STDIO_H_ */
--
2.12.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Proper locking for getchar() and putchar()
2017-08-07 6:33 [PATCH] Proper locking for getchar() and putchar() Sebastian Huber
@ 2017-08-07 9:50 ` Corinna Vinschen
2017-08-07 10:53 ` Sebastian Huber
0 siblings, 1 reply; 6+ messages in thread
From: Corinna Vinschen @ 2017-08-07 9:50 UTC (permalink / raw)
To: newlib
[-- Attachment #1: Type: text/plain, Size: 2351 bytes --]
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?
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Proper locking for getchar() and putchar()
2017-08-07 9:50 ` Corinna Vinschen
@ 2017-08-07 10:53 ` Sebastian Huber
2017-08-07 11:08 ` Corinna Vinschen
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Huber @ 2017-08-07 10:53 UTC (permalink / raw)
To: newlib
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Proper locking for getchar() and putchar()
2017-08-07 10:53 ` Sebastian Huber
@ 2017-08-07 11:08 ` Corinna Vinschen
2017-08-07 11:18 ` Sebastian Huber
0 siblings, 1 reply; 6+ messages in thread
From: Corinna Vinschen @ 2017-08-07 11:08 UTC (permalink / raw)
To: newlib
[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]
On Aug 7 12:53, Sebastian Huber wrote:
> On 07/08/17 11:49, Corinna Vinschen wrote:
>
> > Hi Sebastian,
> >
> > On Aug 7 08:33, Sebastian Huber wrote:
> > > +#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?
I wasn't thinking of simplification. You said it yourself, your
implementation has the benefit of evaluating the reent pointer only
once. While looking into your patch, it immediately struck me as
beneficial to getc/putc, too. So I was thinking of something like
inline _getc_unlocked (_p)
{
struct _reent *_ptr = _REENT;
return (__sgetc_r(_ptr, _p)_;
}
#define getc(_p) _getc_unlocked(_p)
#define getchar() _getc_unlocked(_stdin_r(_REENT))
Does that make sense?
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Proper locking for getchar() and putchar()
2017-08-07 11:08 ` Corinna Vinschen
@ 2017-08-07 11:18 ` Sebastian Huber
2017-08-07 11:57 ` Corinna Vinschen
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Huber @ 2017-08-07 11:18 UTC (permalink / raw)
To: newlib
On 07/08/17 13:08, Corinna Vinschen wrote:
> On Aug 7 12:53, Sebastian Huber wrote:
>> On 07/08/17 11:49, Corinna Vinschen wrote:
>>
>>> Hi Sebastian,
>>>
>>> On Aug 7 08:33, Sebastian Huber wrote:
>>>> +#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?
> I wasn't thinking of simplification. You said it yourself, your
> implementation has the benefit of evaluating the reent pointer only
> once. While looking into your patch, it immediately struck me as
> beneficial to getc/putc, too. So I was thinking of something like
>
> inline _getc_unlocked (_p)
> {
> struct _reent *_ptr = _REENT;
This would be the 2st call to __getreent().
> return (__sgetc_r(_ptr, _p)_;
> }
> #define getc(_p) _getc_unlocked(_p)
> #define getchar() _getc_unlocked(_stdin_r(_REENT))
This would be the 1nd call to __getreent().
>
> Does that make sense?
Not if you want to avoid multiple calls to __getreent().
--
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Proper locking for getchar() and putchar()
2017-08-07 11:18 ` Sebastian Huber
@ 2017-08-07 11:57 ` Corinna Vinschen
0 siblings, 0 replies; 6+ messages in thread
From: Corinna Vinschen @ 2017-08-07 11:57 UTC (permalink / raw)
To: newlib
[-- Attachment #1: Type: text/plain, Size: 2142 bytes --]
On Aug 7 13:18, Sebastian Huber wrote:
> On 07/08/17 13:08, Corinna Vinschen wrote:
>
> > On Aug 7 12:53, Sebastian Huber wrote:
> > > On 07/08/17 11:49, Corinna Vinschen wrote:
> > >
> > > > Hi Sebastian,
> > > >
> > > > On Aug 7 08:33, Sebastian Huber wrote:
> > > > > +#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?
> > I wasn't thinking of simplification. You said it yourself, your
> > implementation has the benefit of evaluating the reent pointer only
> > once. While looking into your patch, it immediately struck me as
> > beneficial to getc/putc, too. So I was thinking of something like
> >
> > inline _getc_unlocked (_p)
> > {
> > struct _reent *_ptr = _REENT;
>
> This would be the 2st call to __getreent().
>
> > return (__sgetc_r(_ptr, _p)_;
> > }
> > #define getc(_p) _getc_unlocked(_p)
> > #define getchar() _getc_unlocked(_stdin_r(_REENT))
>
> This would be the 1nd call to __getreent().
Haha, indeed. Duh! That would ask for an indirection including
the reent pointer as parameter to the _getc_unlocked function.
Well, anyway, go ahead and apply your patch.
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-07 11:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 6:33 [PATCH] Proper locking for getchar() and putchar() Sebastian Huber
2017-08-07 9:50 ` Corinna Vinschen
2017-08-07 10:53 ` Sebastian Huber
2017-08-07 11:08 ` Corinna Vinschen
2017-08-07 11:18 ` Sebastian Huber
2017-08-07 11:57 ` Corinna Vinschen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).