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

[Corinna Vinschen <vinschen at redhat dot com>]

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: signature.asc
Description: PGP signature