Re: more gcc warnings

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Paul Eggert on 7/14/2005 6:03 PM:
> 
> So we don't have to worry about this problem with traditional Unix
> implementations; only with DOS-like ones.
> 
> Is it easy to write an freopen wrapper for cygwin?  And have it do the
> isatty check as well?  That way, we can migrate the "&& ! isatty
> (fileno (stream))" part out of the mailine code.
> 

I tried starting an implementation of rpl_freopen, and it gets complicated
fast (but I think it is doable at least when you are not changing
read/write mode).  If name is not NULL, just return the result of the real
freopen.  From there, use fileno() to convert to an fd (or fail with
EBADF, remembering to call fclose() on all failure paths), and
fcntl(F_GETFL) to see the current mode.  Then you have to parse all 15
valid modes to see which flags to set (or fail with EINVAL).

If all that changed was the addition or subtraction of O_APPEND,
fcntl(F_SETFL) does the trick (while reusing the former values of status
flags such as O_SYNC and O_NONBLOCK).  On cygwin, fcntl(F_SETFL) currently
doesn't change binary vs. text, so that requires setmode(O_BINARY) if the
mode included 'b', and it would be easy to add our !isatty filter here.
But because we do not know whether mode "w" would have opened the current
file in binary or text mode, the best we can do is leave the mode alone
(which means that once rpl_freopen converts a file descriptor to binary,
it cannot restore it back to its default), or else fail with EBADF.  I
don't think we need to support nonstandard modes such as "wt" to force
text mode.

Furthermore, if the requested mode changes the file access, between
O_RDONLY, O_WRONLY, or O_RDWR, then fcntl() is inadequate to perform the
change.  Using fdopen() would handle this, except that it creates a new
FILE* rather than reusing the existing parameter f as freopen requires.
For now, the strictest behavior is to fail with EBADF if the freopen tries
to change the file access mode, which means that freopen(NULL, "rb",
stdin) will fail if stdin was previously opened "r+".  But in reality, it
is probably better that if the new access mode is more restrictive, then
we succeed without changing the mode (for the example of converting stdin
from "r+" to "rb", it means that if we mistakenly call putc() after
rpl_freopen(), it will succeed instead of fail).  Still, we are forced to
return EBADF if the request was to add an access mode.  Also, when mode
includes 'w', we would have to call ftruncate().

Now if we don't mind changing POSIX semantics, by leaving f unchanged (and
open) on failure and returning a different FILE* on success than f, then
it is a simple matter of making rpl_freopen defer to fdopen (fileno (f),
mode).  If the fdopen fails, we just return NULL, and the paradigm becomes:

if (O_BINARY)
  {
    FILE *tmp = freopen (NULL, "rb", stdin);
    if (tmp) stdin = tmp;
  }

That way, a failure in freopen does not corrupt stdin, which is essential
if the rest of the utility is to stand a chance of proper operation.  With
the current CVS code, the EFAULT failure of cygwin's freopen is being
ignored, but cygwin closed stdin in the attempt, so that all future file
operations fail with EBADF:
$ echo hi | coreutils/src/tr a b
coreutils/src/tr: read error: Bad file descriptor

Hmm, maybe it is simpler, after all, to stick with the nonstandard setmode
(fileno (f), O_BINARY) than to try to get a working freopen, or making
freopen semantics change from the standards to fit our needs and hoping
that we don't ever add a future use that would be broken by our changed
semantics.  Even if newlib (the provider of freopen() for both cygwin and
mingw) updates freopen() to actually implement file access changes where
possible, we still need to deal with the fact that a failure will close
the underlying file descriptor.

Hopefully we are safe so long as we limit ourselves to just use
freopen(NULL, "rb", stdin), freopen(NULL, "wb", stdout), and
freopen(non_null, any_mode, any_file), as is the current case in CVS.

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFC17wY84KuGfSFAYARAgqCAJsFM6SAiEbcsbZnp7nWBvTrEBZ6XwCfW2xA
MnFdFx9H6RE4vlx4+w4PWJ4=
=3i7n
-----END PGP SIGNATURE-----