Re: [PATCH v3] libio: do not unbuffer legacy standard files in compatibility mode [BZ #24228]

["Dmitry V. Levin" <ldv at altlinux dot org>]

On Wed, Mar 13, 2019 at 04:49:59PM +0100, Florian Weimer wrote:
> * Dmitry V. Levin:
> 
> > On Wed, Mar 13, 2019 at 04:08:16PM +0100, Florian Weimer wrote:
> >> * Dmitry V. Levin:
> >> 
> >> > Commit glibc-2.23~693 (a601b74d31ca086de38441d316a3dee24c866305)
> >> 
> >> Please quote the commit hash and commit subject, kernel-style.
> >
> > The kernel-style reference would look this way:
> >
> > Commit a601b74d31ca ("In preparation for fixing BZ#16734, fix failure in
> > misc/tst-error1-mem when _G_HAVE_MMAP is turned off.")
> >
> > I'd like to add a reference to glibc-2.23~693 somewhere because
> > I find it useful, but I don't see any suitable place for it
> > in this long kernel-style form.
> 
> Hmm.  It's not strictly speaking unique because we do not reject
> branches, unfortunately.

No, it's not unique so I also add the commit hash, but it's handy.

> >> (How did you determine this reference, anyway?)
> >
> > Sorry?
> 
> glibc-2.23~693 looks nice, but I can't get “git describe” to produce.

git describe --contains a601b74d31ca

> >> > diff --git a/libio/genops.c b/libio/genops.c
> >> > index 2a0d9b81df..aa92d61b6b 100644
> >> > --- a/libio/genops.c
> >> > +++ b/libio/genops.c
> >> > @@ -789,6 +789,10 @@ _IO_unbuffer_all (void)
> >> >  
> >> >    for (fp = (FILE *) _IO_list_all; fp; fp = fp->_chain)
> >> >      {
> >> > +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> >> > +      if (__glibc_unlikely (&_IO_stdin_used == NULL) && _IO_legacy_file (fp))
> >> > +	continue;
> >> > +#endif
> >> 
> >> I wonder if we should check _IO_legacy_file only here.  This is related
> >> to the previous discussion.
> >
> > If we omitted the check for _IO_stdin_used, then standard files
> > would be skipped and misc/tst-error1-mem would complain.
> 
> Really?  Why would _IO_legacy_file be true for those?  That's definitely
> not what I intended.

I don't see why it shouldn't be the case, but it was a month ago
when I looked at the code, so I'll re-check.


-- 
ldv

Attachment: signature.asc
Description: PGP signature