Re: [PATCH] xfreopen: keep append mode when converting stdout to binary

[Bruno Haible]

Hi Eric,

> For a long time, we've had reports against coreutils about improper
> use of freopen(NULL) on platforms with non-zero O_BINARY: basically,
> when built out of the box, things like 'cat a >> b' would overwrite
> data in b, because the freopen(NULL,"wb",stdout) lost the O_APPEND
> flag.

I don't think this functionality belongs in the xfreopen() function.
xfreopen() is defined as freopen() + error checking. Making its semantics
different from the one of freopen() will provoke bugs in some callers
in the future. If someone passes the mode = "wb" argument, he certainly
expects to use the open() flags O_WRONLY|O_CREAT|O_TRUNC, as listed in POSIX
<http://www.opengroup.org/onlinepubs/9699919799/functions/freopen.html>.
No one will understand, when reading code like the one in src/head.c:

  if (O_BINARY && ! isatty (STDOUT_FILENO))
    xfreopen (NULL, "wb", stdout);

that this may use "ab" instead of "wb".

I would find it better to leave xfreopen alone and add a function

  /* Returns the current mode of FP, combined with the additional
     open_flags.  Allowed open_flags are O_APPEND, O_TEXT, O_BINARY,
     or a combination of these.  */
  const char *fgetmode (FILE *fp, int open_flags)

Then src/head.c is changed to:

  if (O_BINARY && ! isatty (STDOUT_FILENO))
    xfreopen (NULL, fgetmode (stdout, O_WRONLY | O_BINARY), stdout);

Or, alternatively, add a new function [x]fchangemode that relies on
[x]freopen:

  /* Changes the mode of FP, but keeping those flags that are set
     in kept_flags and adding flags that are set in added_flags.
     Allowed values for kept_flags and added_flags are O_RDONLY,
     O_WRONLY, O_RDWR, O_APPEND, O_TEXT, O_BINARY, or a combination of
     these.  */
  void fchangemode (FILE *fp, int kept_flags, int added_flags);

Then change src/head.c to read:

  if (O_BINARY && ! isatty (STDOUT_FILENO))
    xfchangemode (stdout, O_APPEND, O_WRONLY | O_BINARY);

Note how this is different from freopen: freopen (NULL, "wb", stdout)
would be equivalent to
    fchangemode (stdout, 0, O_WRONLY | O_BINARY);

Bruno