bug#25707: [PATCH] grep: don't forcefully strip carriage returns

[Eric Blake]

On 02/14/2017 05:08 PM, Paul Eggert wrote:
> On 02/13/2017 12:20 PM, Eric Blake wrote:
>> undossify_input causes more problems than it
>> solves.  We should trust fopen("r") to do the right thing, rather than
>> reinventing it ourselves.
> 
> Yes, that makes sense. Attached is a proposed patch to implement this.
> It assumes the patch you already submitted for Bug#25707.
> 
> This patch keeps the -U option, for MS-Windows users who want to
> override fopen "-r"'s choice of binary vs text I/O. Perhaps that's too
> conservative? It would be easy to turn -U into a no-op too.

No, keep -U exactly as proposed in the patch (sometimes you WANT to
force binary mode - forcing binary is a no-op on Unix platforms, but
makes sense on Windows).  I like the patch as you wrote it, modulo nits:

> .
> -This option has no effect
> -on platforms other than MS-DOS and MS-Windows.
> address@hidden MS-Windows binary I/O
> address@hidden binary I/O, /MS-Windows

Why the slash before MS?

> 
> +
> +This option has no effect on platforms other than MS-Windows.

There are other systems than MS-Windows that have non-zero O_BINARY.  I
don't know if you want to reword this a bit, to maybe state that the
option has no effect on platforms where binary mode is identical to text
mode.  I guess your wording is fine, though.

> 
> @@ -1862,7 +1856,7 @@ grepdesc (int desc, bool command_line)
>  
>    /* Set input to binary mode.  Pipes are simulated with files
>       on DOS, so this includes the case of "foo | grep bar".  */
> -  if (O_BINARY && !isatty (desc))
> +  if (binary && !isatty (desc))
>      set_binary_mode (desc, O_BINARY);
>  

The comment is somewhat stale; pipes are not simulated in MS-Windows,
and these days ports to older DOS are rare (is DJGPP still viable?).
Probably safe to just delete the second sentence.

> 
>    /* Output is set to binary mode because we shouldn't convert
>       NL to CR-LF pairs, especially when grepping binary files.  */
> -  if (O_BINARY && !isatty (STDOUT_FILENO))
> -    set_binary_mode (STDOUT_FILENO, O_BINARY);
> +  if (binary && !isatty (STDOUT_FILENO))
> +    xfreopen (NULL, "wb", stdout);

xfreopen() may need a patch - if stdout was previously opened in append
mode, this reopen breaks that.  Several of the coreutils are affected by
the same problem; here's the patch I've been applying when building
coreutils for Cygwin:

 void
 xfreopen (char const *filename, char const *mode, FILE *fp)
 {
+  if (!filename && STREQ (mode, "wb"))
+    {
+      int flag = fcntl (fileno (fp, F_GETFL);
+      if (0 <= flag && (flag & O)APPEND))
+        mode = "ab";
+    }
   if (!freopen (filename, mode, fp))

I guess I should push that one into gnulib.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature