This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Why fixing 9533 doesn't fix 7744: revealed!


In article <3E666428 dot 8040004 at unitus dot it> Paolo, the new wonder bug
fixer and performance enhancer, writes:

>> The call to xsgetn is at the wrong level of abstraction.  It keeps
>> trying until it gets n characters, or EOF.  You want to call ::read()
>> in underflow(), and just go with as many characters as it returns.
>> (If it returns -1 and errno is EINTR you would call it again.)
>> Then, get rid of that isatty()!

Hi Paolo,

I agree with the above in theory.  And, I would fully support seeing
it in practice whenever ios::sync_with_stdio(false) had been called.
However there are a few issues to consider (I'm sure you have already
spotted them, but I want them filed for the record before you generate
a patch): underflow()'s implementation is currently declared where it
is *not* legal to put direct calls to ::read().  IMHO, you will have
to rearrange its body or delegated body to appear in
config/io/bla-bla.  Nathan/Benjamin, confirm that you'd agree with
this.  Either way, I'd agree that the standard says nothing about
underflow() having to be implemented in terms of xsgetn().  In
retrospect, that was clearly an unfortunate implementation choice.
I.e. good catch Nathan.  All the times I've hacked on that code, I
took it as a given that underflow had to be implemented in such terms!

> Ok (too bad that I learned how to use some autoconf machinery for
> something which was not going to last much ;)

> To be honest, at the time Loren envisaged using read (and, at the time
> I didn't realy understand that :) but had also some perplexities:

Mentioning my name in a thread will *not* get me involved beyond
commentary... ;-)

> >... POSIX IO. Someone skilled in the art could construct a
> >well-performing implementation of __basic_file<char>::xsgetn() that
> >could return short-reads [e.g. fread() (stdio) will only return a
> >short-read on error or EOF whereas read() (POSIX) will short-read
> >based on the current line discipline]. It is possible that this
> >alternate implementation should only be used when
> >ios::sync_with_stdio(false) was called (since the only portable way to
>                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >share stdio buffers is to make stdio calls).
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Do you believe I can go ahead and safely change the underlying
> implementation of xsgetn to use read instead of fread?

For all cases?  No, but I'm willing to be shown how.  If you can find
a way to unconditionally change the underlying implementation, then
feel free to undo all the hacks we installed to fix various corner
cases.  (i.e. I can't disagree with Nathan's assessment.)  OTOH, I
feel it would be a clear win to improve the
ios::sync_with_stdio(false) case through various means even if the
ios::sync_with_stdio(true) case continues to require special hacks.

Regards,
Loren


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]