[patch, libgfortran] PR47694 [4.3/4.4/4.5/4.6 Regression] Fortran read from named pipe fails

[Jerry DeLisle]

On 02/19/2011 10:27 AM, Jerry DeLisle wrote:
> On 02/19/2011 09:15 AM, Janne Blomqvist wrote:
>> On Sat, Feb 19, 2011 at 04:18, Jerry DeLisle<jvdelisle@frontier.com> wrote:
>>> Hi folks,
>>>
>>> The problem here is that when a read from a named pipe reads more than one
>>> line worth, the formatted IO buffer holds onto the data. When fbuf_read is
>>> subsequently invoked, a sread is dutifully called and it waits for the next
>>> read completion. In the meantime, the next line of data is already in the
>>> buffer so the read waits for another "enter". The reads get out of sync.
>>> The problem does not occur with regular files because there is either
>>> always data available from the file to read or an EOF occurs and in both
>>> those cases sread returns.
>>
>> Thanks for the explanation; So I think the fundamental issue is that
>> in order to handle pipes (and perhaps other stuff like terminals too?)
>> correctly, we need to consume all the bytes in the buffer before
>> asking for more via sread.
>>
>>> The patch fixes this by scanning the buffer under certain conditions looking
>>> for an End-of-Line (or EOR). If EOR is found, there is no need to sread
>>> again from the pipe. This patch does add a little overhead from the tight
>>> scan loop, but will save some system calls.
>>>
>>> Regression tested on x86-64. I would appreciate others testing please. I do
>>> not know how to create a specific named pipe test in the test suite. See the
>>> PR for the original test.
>>>
>>> OK for trunk?
>>
>> if (oldpos + *len> oldact)
>> {
>> + /* See if we have a pending EOR in here. */
>> + if (*len> oldact)
>>
>> This second conditional makes no sense. What matters is if the current
>> position in the buffer + len goes past the active bytes, which is
>> exactly the previous conditional. Thus, does the patch work if you
>> remove that conditional and check for EOR unconditionally if oldpos +
>> *len> oldact?
>
> I thought it made no sense either. But without it, fmt_t_2.f90 fails. It is the
> only regression I found when testing this patch. I determined that condition by
> experiment. I looked at the values of oldpos, oldact, *len for various test
> cases to see what was unique about that one in particular. I then tried
> different combinations of conditions until it worked for the piped case and this
> case and no regressions. I suppose we could study that case more explicitly, but
> it is unique. Maybe a bug elsewhere?
>
>>
>> Secondly, you check for "\n" or "\r", but I think you need to check
>> that a "\r" is followed by a "\n" before you can say that you're at
>> EOR.
>>
>
> It does not matter because either way the result is we read what is in the
> buffer and sf_read looks for the \r followed by \n. All we are doing here is
> avoiding an sread and I wanted to address the case where there is a \r without a
> \n. Of course I just realized I can artificially test this on my machine here
> and I will do so.
>

Well, I tested.  On my system, \r by itself works, \n by itself works, \r\n 
works, but my system sees it as two lines and not one.  That may be normal.  I 
will have to get it tested on a cr-lf based system.

What do you think?

Jerry