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.
> +@cindex MS-Windows binary I/O
> +@cindex 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
