bugfindutils - Bugs: bug #27983, Windows text mode mis-interpreted

 
 

bug #27983: Windows text mode mis-interpreted

Submitter:  Ilguiz Latypov <ilgiz>
Submitted:  Tue 10 Nov 2009 04:04:14 AM UTC
 
Category:  xargs Severity:  3 - Normal
Item Group:  None Status:  Wont Fix
Privacy:  Public Assigned to:  ericb
Originator Name:  Open/Closed:  Closed
Release:  None Fixed Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Mon 12 Apr 2010 09:13:35 PM UTC, comment #11: 

I have no qualms with using 'b' - on a POSIX system it has no effect, but we know that on non-POSIX systems, it is standardized by C99 and has a beneficial effect.

But I am adamantly opposed to using 't' - it is not specified by any standard, and creates more problems than it solves.

If you have complaints about how cygwin handles newlines, take it up on the cygwin list, not here.  I have no plans on fixing it upstream in findutils, since it is not an upstream findutils issue.  But since it affects cygwin users, you may find more agreement on the cygwin list, or better ways to accomplish what you are trying to do by patching xargs.  And I disagree with your statement that cygwin apps cannot rely on POSIX compliance of the system features they consume - if you use only cygwin apps and binary mount points (which is the only environment where cygwin makes promises of POSIX compliance, anyways), then there is no problem.  And if you use anything else, then you only have to isolate the differences at the boundaries, as with the d2u trick.

Eric Blake <ericb>
Project Administrator
Mon 12 Apr 2010 08:32:07 PM UTC, comment #10: 

> Cygwin cannot be POSIX-compliant


I meant "Cygwin applications cannot rely on POSIX compliance of the system features they consume".

Ilguiz Latypov <ilgiz>
Mon 12 Apr 2010 08:29:10 PM UTC, comment #9: 

> app | d2u | xargs


> That is a one-size-fits-all solution - it works no matter the windows app on the left and no matter the consumer on the right.


This means that the developers should be aware about the newline subtlety that may be exposed by the use of Windows native applications and the Cygwin's text mode mounts.

Some developers may test their changes on binary only mounts forgetting about other users who are not even aware about the kind of the mount they have.

> to make it easier to change an existing stream to binary-only


I do not understand your point.  Earlier you argued that using the 'b' option in the file open mode makes no sense on POSIX, and this is confirmed by the Open Group specs.

Now you are saying that since the use of that option should be harmless on POSIX-compliant systems, we could use it so that on some specific systems this option would make a difference. 

But this hides the fact that Cygwin cannot be POSIX-compliant when processing arbitrary input streams or generating output streams.  This is because Cygwin should take the notion of native newlines into account.

As for the suggested change to either the use of freopen() or to the xfreopen() wrapper with regards to preserving the possible append mode, I agree that all uses of freopen() with "w" may need reviewing.  I am afraid that just modifying freopen()'s implementation itself to preserve the append mode would be too drastic as this could change the existing behavior where the append mode was not supposed to be preserved.

(In case of xargs, only the mode of the input stream needs to be changed, so preserving the append mode does not seem to be an issue to me).

Ilguiz Latypov <ilgiz>
Mon 12 Apr 2010 03:48:55 PM UTC, comment #8: 

If you really want xargs to handle data from a native windows application, then add d2u into the pipeline:

app | d2u | xargs

That is a one-size-fits-all solution - it works no matter the windows app on the left and no matter the consumer on the right.  And since it works, I see no reason to teach xargs, and every other potential consumer on the right, to be tolerant, when you can merely insert d2u to convert from one paradigm to the other.  That is, xargs is just fine living in a paradigm of POSIX binary-only behavior, and if you need to interact with text mode, limit the interaction to the boundary - I still don't see why xargs should ever force text.

On the other hand, using the 'force binary' hint, particularly in concert with -0, is definitely worth including in xargs proper.  However, rather than making the patch hard-coded on #ifdef _CYGWIN_, it would be better to help gnulib introduce a module (or enhance the existing binary-io module) to make it easier to change an existing stream to binary-only.  I've mentioned it before, and even submitted potential patches, but have not yet had time to complete a solution that everyone was happy with: http://lists.gnu.org/archive/html/bug-gnulib/2010-03/msg00111.html.  Once we have that solution in gnulib, then your patch to reopen input_stream in binary mode makes sense.

Eric Blake <ericb>
Project Administrator
Fri 09 Apr 2010 11:47:14 PM UTC, comment #7: 

xargs may receive input not just from a text-mode-mounted file but also from a Windows native application.  So I thought that being liberal on input was good and gave the "t" hint to Cygwin DLL in my added freopen().

In the case of "-d X" or "-0" options xargs expects raw input stream so I gave the "force the binary mode" hint to Cygwin DLL in another call to freopen().

I agree these hints contradict the POSIX environment.  But the very notion of CR/NL line endings may also be against POSIX as well.  It appears Cygwin was supposed to make this difference less visible, but I see that some may regard Cygwin as a platform for Cygwin-aware programs only.

Ilguiz Latypov <ilgiz>
Fri 09 Apr 2010 04:57:29 PM UTC, comment #6: 

In the POSIX world, fopen() with 'b' has no effect, but with 't' has undefined behavior ('t' happens to be a cygwin extension, and you have no business trying to use it in an application trying to be portable to POSIX).

I still argue that the problem is not in findutils, but in your (mis)use of cygwin text mounts.  Cygwin recommends using binary mounts, not text mounts, for a reason - because apps written for POSIX expect POSIX behavior.  And when you violate that assumption, the burden is on you, not on the apps.  If \r through pipes is a problem, then add d2u into the pipeline - that way, you have a single tool and a paradigm that can fix the issue for every situation, rather than having to patch every downstream tool to understand a new paradigm.

So findutils' testsuite won't pass on a cygwin text mount.  I don't care, as long as it continues to pass on a cygwin binary mount.

Eric Blake <ericb>
Project Administrator
Fri 09 Apr 2010 02:32:04 AM UTC, comment #5: 

> Then you use cat, which preserves binary mode, through a pipe which also preserves binary mode.


But this is a valid sequence of commands, and Cygwin should preserve the behavior of this sequence regardless of the input's low-level line endings, as long as the latter are in accord with the text mode detection rules.  Pipes are the corner case and this is why I appeal to force explicit input conversion in Cygwin's xargs.

The attached patch made the CRs disappear on reading the pipe.

I also realized that the "t" and "b" options in the fopen() mode argument are supposed to be irrelevant in the POSIX world. 

Here are the outputs of the improved test script.

Before the patch:

$ PATH="/usr/src/findutils-4.5.5-1/build/xargs:${PATH}" ./text-mode.sh
umount: /textmode: Invalid argument
mount: warning - /textmode does not exist.
CR found in /textmode/file1.txt.

      • Unexpectedly, CR found in /textmode/file2.txt.

CR found in /textmode/file3.txt.

      • Unexpectedly, CR not found in /textmode/file4.txt.

CR not found in /textmode/file5.txt.
CR not found in /textmode/file6.txt.

After the patch:

$ PATH="/usr/src/findutils-4.5.5-1/build/xargs:${PATH}" ./text-mode.sh
umount: /textmode: Invalid argument
mount: warning - /textmode does not exist.
CR found in /textmode/file1.txt.
CR not found in /textmode/file2.txt.
CR found in /textmode/file3.txt.
CR found in /textmode/file4.txt.
CR not found in /textmode/file5.txt.
CR not found in /textmode/file6.txt.

(file #20165, file #20166)

Ilguiz Latypov <ilgiz>
Mon 05 Apr 2010 03:14:51 PM UTC, comment #4: 

In your example, $tf1 is created in text mode, so it contains file names listed with carriage returns.  Then you use cat, which preserves binary mode, through a pipe which also preserves binary mode.  But this is an example of a useless use of cat.  You could have instead used:

xargs -i echo -n "=={}==" <$tf1 >$tf2

which would have opened tf1 in text mode (per the mount point), and avoided the wasted cat process in the first place.  If you insist on using cat, then you should also be able to use d2u:

cat "$tf1" | d2u | xargs -i echo -n "=={}==" >$tf2

I still see no reason for findutils to work around this issue, especially given the fact that cygwin developers highly recommend binary mounts rather than text mounts for the very reason that text mounts are non-POSIX and receive less testing.

Eric Blake <ericb>
Project Administrator
Mon 05 Apr 2010 02:56:55 PM UTC, comment #3: 

Is the expectation in the test script valid?

> mount -t "$(cygpath -w "${dir}")" /textmode

[..]

> tf1="/textmode/file1.txt"
> tf2="/textmode/file2.txt"

[..]

> echo "foo" > "${tf1}"
> echo "bar" >> "${tf1}"

[..]

> cat "${tf1}" | xargs -i echo -n "=={}==" > "${tf2}"

[..]

> find_crs "${tf2}"   # CR found (Savannah bug 27983 in xargs 4.4.0)

Ilguiz Latypov <ilgiz>
Mon 05 Apr 2010 12:45:57 PM UTC, comment #2: 

Cygwin does provide the ability to request explicit text-mode processing, but it is almost always a bug to use it.  For example, it is possible to name files with an explicit trailing carriage return; blindly using text mode would make it impossible to process these files through xargs.  Rather, programs should trust that for text files, plain "r" will do the right thing given the underlying mount point.  Input through a pipe from a cygwin process will already be in binary mode and not have spurious carriage returns.  And if you are using input from a non-cygwin program, then you can simply add another process in the pipeline, and pass your data through d2u, rather than having to patch every other application to have to care about the differences.

As maintainer of the cygwin port of findutils, I don't think that this should be changed in findutils.

Meanwhile, I don't see what your complaint is about the xfreopen wrapper; but if you think there is a bug with it, you should report it to the gnulib list.  POSIX requires that freopen return either NULL on failure, or the original file argument on success - there is no implementation that returns a different file than what you pass in.

Eric Blake <ericb>
Project Administrator
Tue 17 Nov 2009 12:30:53 AM UTC, comment #1: 

Correction of my phrase on the extra file entity,

  One has to explicitly set it with the "r" vs. "rb" (O_BINARY) open mode.

There is no explicit text mode setting.  The "r" mode is the default and will be interpreted by the emulation layer as a text or binary mode using heuristic.

Ilguiz Latypov <ilgiz>
Tue 10 Nov 2009 04:04:14 AM UTC, original submission:  

When xargs receives its input from a pipe or redirect, it uses the stdin file descriptor prepared for it by the OS environment.

Emulation of Unix on Windows introduces an extra entity into file operations, text vs. binary mode processing.  One has to explicitly set it with the "r" vs. "rb" (O_BINARY) open mode.

The Unix-on-Windows OS environment has to rely on heuristic such as the mount flag "binmode" and CYGWIN variable's "binmode" element to decide on the default mode for stdin and stdout file descriptors. 

I am not sure if the implementation of "r" and "w" modes in fopen() calls also relies on this heuristic, but this could be verified by running my script before and after the change.

It appears that there is not such thing as "rt" and "wt" modes in the Open Group standard.  These would be nice to turn the text mode processing explicitly.

  http://www.opengroup.org/onlinepubs/000095399/functions/fopen.html

I believe text processing utilities that care about the Unix-on-Windows emulations need an option to specify input/output modes explicitly or, at least, to declare a certain mode of operation explicitly.  The "sed" utility has a "--binary" option and other utilities such as "wc" and "tr" have switched to the certain binary mode.

With xargs, the heuristic rules of choosing the text processing mode for the input stream work in synchronization with other tools producing such streams.

There is a corner case where a text stream is obtained from a pipe.  I believe pipes are always opened in binary mode by the Unix-on-Windows environment as it is difficult to predict the kind of processing that will consume the pipe.

I am attaching a test script that shows how xargs fails to set the text processing mode on standard input obtained from a pipe.  I believe this could be done in a way similar to the "tr" implementation but with the opposite intention,

  stdin = freopen( NULL, "r", stdin );

  http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=blob;f=gl/lib/xfreopen.c;h=32e68fa35c8f8cc5c95641a0fc3b761d23d6bf8d;hb=HEAD

  http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=blob;f=src/tr.c;h=c5f18f98384e10ff871a4e4daac42270e96743d6;hb=HEAD

  http://www.opengroup.org/onlinepubs/000095399/functions/freopen.html

(The xfreopen wrapper ignores the possibility that freopen may return a different pointer, so I would not recommend using this wrapper).

Ilguiz Latypov <ilgiz>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #20166:  text-mode.sh added by ilgiz (5KiB - text/x-sh - a suggested Cygwin patch to xargs-4.5.5-1)
file #20165:  xargs-4.5.5-1-explicit-text-or-binary-mode.txt added by ilgiz (883B - text/plain - a suggested Cygwin patch to xargs-4.5.5-1)
file #19024:  text-mode.sh added by ilgiz (3KiB - text/x-sh - update 2: clarify comments)
file #19023:  text-mode.sh added by ilgiz (3KiB - text/x-sh - update: generate same input as a point of reference)
file #19022:  text-mode.sh added by ilgiz (3KiB - text/x-sh - A script showing how xargs 4.4.0 accepts the existing text processing mode of stdin)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jay (Updated the item)
  • -email is unavailable- added by ericb (Posted a comment)
  • -email is unavailable- added by ilgiz (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

     

     

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-05-11 jay Open/ClosedOpen Closed
    2010-04-09 ilgiz Attached File- Added text-mode.sh, #20166
    2010-04-09 ilgiz Attached File- Added xargs-4.5.5-1-explicit-text-or-binary-mode.txt, #20165
    2010-04-05 ericb StatusNone Wont Fix
        Assigned toNone ericb
    2009-11-10 ilgiz Attached File- Added text-mode.sh, #19024
    2009-11-10 ilgiz Attached File- Added text-mode.sh, #19023
    2009-11-10 ilgiz Attached File- Added text-mode.sh, #19022

    Back to the top


    Powered by Savane 3.9