Re: [PATCH x 5] warnings, some legit

[Eric Blake]

Jim Meyering <jim <at> meyering.net> writes:

> At first I was tempted to simply ignore any freopen failure,
> thinking that it was so unlikely as to be truly ignorable.
> And in addition, most are guarded by "if (O_BINARY &&...",
> so wouldn't even be compiled on systems I care about.
> However, there are actually many ways in which freopen can
> fail, so I wrote xfreopen and used that everywhere.

I'll have to give it a whirl on cygwin, since that is affected most by the 
freopen uses.  For that matter, at one point I thought about adding a gnulib 
module to make reopening a stream in binary mode a little less painful; the 
current freopen trick has to query whether stdout is in append mode and choose 
between "w" and "a" accordingly when reopening stdout.

> --- a/src/copy.h
> +++ b/src/copy.h
> @@ -221,6 +221,12 @@ struct cp_options
>     ? lstat (Src_name, Src_sb) \
>     : stat (Src_name, Src_sb))
> 
> +static inline void
> +ignore_value (int i ATTRIBUTE_UNUSED)

Is this something worth putting in system.h, or even in a gnulib module, rather 
than buried in copy.h?

> 
> +#define TMP (char *) "/tmp"

gcc doesn't warn about this use?  Oh well, whatever works.

> +void
> +xfreopen (char const *filename, char const *mode, FILE *fp)
> +{
> +  if (!freopen (filename, mode, fp))
> +    error (exit_failure, errno, _("failed to reopen %s"), quote (filename));

Bug.  filename is allowed to be NULL (most often the case with stdin/stdout).

> --- a/src/cat.c
> +++ b/src/cat.c
> @@ -39,6 +39,7 @@
>  #include "full-write.h"
>  #include "quote.h"
>  #include "safe-read.h"
> +#include "xfreopen.h"
> 
>  /* The official name of this program (e.g., no `g' prefix).  */
>  #define PROGRAM_NAME "cat"
> @@ -664,7 +665,7 @@ main (int argc, char **argv)
>      {
>        file_open_mode |= O_BINARY;
>        if (O_BINARY && ! isatty (STDOUT_FILENO))
> -     freopen (NULL, "wb", stdout);
> +     xfreopen (NULL, "wb", stdout);

Yep, this is one of those places where a blind freopen(NULL,"wb",stdout) is 
wrong if stdout is originally in append mode, and where you would expose the 
xfreopen bug.  I need to revisit some of my cygwin-specific patches and push 
them upstream.

-- 
Eric Blake