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

[Janne Blomqvist]

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?

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.

I wonder, would it perhaps be cleaner to create a new function, say
something like

static inline uchar *
fbuf_getptr (gfc_unit * u)
{
  return (uchar*) u->fbuf->buf + u->fbuf->pos;
}

use this in read_sf() to get a pointer to eventually return, then loop
forwards one character at a time using fbuf_getc() checking for
EOR/EOF and other conditions/errors along the way.

What do you think?


-- 
Janne Blomqvist