Re: cat bug on cygwin

[Jim Meyering]

Eric Blake <address@hidden> wrote:
...
> I'm wondering if we could just add something like this to the binary-io
> module, which we can then update as needed to compensate for any other
> irregularities discovered in swapping stdio to binary mode:

I like it.
I know the code below is just a sketch, but here's some feedback.

fsetbinary (fp) can be defined to just "(fp)" when O_BINARY is 0 or
undefined.  Then, we can clean up related code in coreutils like this:

diff --git a/src/cksum.c b/src/cksum.c
index 7b65c73..4bd05df 100644
--- a/src/cksum.c
+++ b/src/cksum.c
@@ -194,14 +194,12 @@ cksum (const char *file, bool print_name)

   if (STREQ (file, "-"))
     {
-      fp = stdin;
+      fp = fsetbinary (stdin);
       have_read_stdin = true;
-      if (O_BINARY && ! isatty (STDIN_FILENO))
-       freopen (NULL, "rb", stdin);
     }
   else
     {
-      fp = fopen (file, (O_BINARY ? "rb" : "r"));
+      fp = fsetbinary (fopen (file, "r"));
       if (fp == NULL)
        {
          error (0, errno, "%s", file);

Of course, that means your fsetbinary has to return NULL
immediately when the input FP is NULL.

> /* Make sure FP is in binary mode.  Return FP on success,
>    NULL on failure.  */
> FILE *
> fsetbinary (FILE *fp)
> {

Insert this (after hoisting decls, I presume):

    if (fp == NULL)
      return NULL;

I'd tend to not set errno, on the presumption that
it was set already by the surrounding code (as in the cksum/fopen
case above), but that isn't necessarily the case.
Something to think about...

> #ifdef __CYGWIN__
>   int mode = fcntl (fileno (fp), F_GETFL);
>   char *str;

char const *str;

>   switch (mode & (O_ACCMODE | O_APPEND))
>     {
>     case O_RDONLY: str = "rb"; break;
>     case O_WRONLY: str = "wb"; break;
>     case O_RDWR: str = "r+b"; break;
>     case O_WRONLY | O_APPEND: str = "ab"; break;
>     case O_RDWR | O_APPEND: str = "a+b"; break;
>     default: str = NULL;
>     }
>   if (!str)
>     {
>       fclose (fp);
>       errno = EBADF;
>       return NULL;
>     }
> # ifdef simple
>   return freopen (NULL, str, fp);
> # else
>   fp = freopen (NULL, str, fp);
>   if (fp)
>     {
>       fcntl (fileno (fp), F_SETFL, mode);

I assume this is a "can't fail" situation for Cygwin, too.

>     }
>   return result;

s/result/fp/

> # endif
> #else
>   /* I'm not sure how mingw behaves with freopen(NULL,...).  */
>   SET_BINARY (fileno (fp));
>   return fp;
> #endif
> }