[Bug fortran/47694] [4.3/4.4/4.5/4.6 Regression] Fortran read from named pipe fails to read all available data

["jb at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47694

--- Comment #21 from Janne Blomqvist <jb at gcc dot gnu.org> 2011-02-24 15:04:42 UTC ---
(In reply to comment #20)
> Some random thoughts looking at the patch
> (http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170432) - probably not
> helping at all with the regressions:
> 
> I find the following a bit dubious - thought it was also odd before:
> 
> +  base = fbuf_getptr (dtp->u.p.current_unit);
>    if (base == NULL)
>      return NULL;
> 
> The first line is effectively:
>   base = (char*) (u->fbuf->buf + u->fbuf->pos);
> 
> Thus, base is only NULL if "u->fbuf->buf" == NULL *and* u->fbuf->pos == 0. I am
> not sure about the second part, though I think in practice, fbuf_init has
> (hopefully!) always been called and thus it will never be NULL.

I agree, that NULL check can be removed.

> 
> 
> +      if (q == EOF)
> 
> Does this always work? We have in "static inline int fbuf_getc (gfc_unit * u)":
>     return (unsigned char) u->fbuf->buf[u->fbuf->pos++];
> and "q" is "int".
> 
> The issue I see is the following (quote from POSIX's fgetc(3p)):
> "If the integer value returned by fgetc() is stored into a variable of type
> char and then compared against the  integer constant EOF, the comparison may
> never succeed, because sign-extension of a variable of type char on widening to
> integer is implementation-defined."

You're missing the whole picture. The definition of fbuf_getc in fbuf.h is

static inline int
fbuf_getc (gfc_unit * u)
{
  if (u->fbuf->pos < u->fbuf->act)
    return (unsigned char) u->fbuf->buf[u->fbuf->pos++];
  return fbuf_getc_refill (u);
}

That is, the inlined fast path is for the case when there is data available in
the buffer. If there isn't data, it falls back to fbuf_getc_refill, where if
filling the buffer with more data fails EOF is returned, otherwise the next
character cast to unsigned char. 

FWIW, if you dig into the glibc sources, the getc() macro is implemented in a
very similar way.