From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#18006: Simplify via set_binary_mode Date: Mon, 14 Jul 2014 18:21:49 +0300 Message-ID: <83pph87ype.fsf@gnu.org> References: <53C1A00C.6050906@cs.ucla.edu> <837g3hanzp.fsf@gnu.org> <53C21918.9020001@cs.ucla.edu> <8338e59u9u.fsf@gnu.org> <53C33C6A.6060601@cs.ucla.edu> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1405351353 17653 80.91.229.3 (14 Jul 2014 15:22:33 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 14 Jul 2014 15:22:33 +0000 (UTC) Cc: 18006@debbugs.gnu.org To: Paul Eggert Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Jul 14 17:22:24 2014 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1X6i5A-0006n7-2n for geb-bug-gnu-emacs@m.gmane.org; Mon, 14 Jul 2014 17:22:24 +0200 Original-Received: from localhost ([::1]:58374 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X6i59-00043k-Jp for geb-bug-gnu-emacs@m.gmane.org; Mon, 14 Jul 2014 11:22:23 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:33581) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X6i4u-0003nA-K5 for bug-gnu-emacs@gnu.org; Mon, 14 Jul 2014 11:22:14 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X6i4o-0008Ky-Ec for bug-gnu-emacs@gnu.org; Mon, 14 Jul 2014 11:22:08 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:59524) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X6i4o-0008Kn-By for bug-gnu-emacs@gnu.org; Mon, 14 Jul 2014 11:22:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1X6i4n-00056H-VK for bug-gnu-emacs@gnu.org; Mon, 14 Jul 2014 11:22:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 14 Jul 2014 15:22:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 18006 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 18006-submit@debbugs.gnu.org id=B18006.140535131219582 (code B ref 18006); Mon, 14 Jul 2014 15:22:01 +0000 Original-Received: (at 18006) by debbugs.gnu.org; 14 Jul 2014 15:21:52 +0000 Original-Received: from localhost ([127.0.0.1]:54790 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1X6i4a-00055i-Jj for submit@debbugs.gnu.org; Mon, 14 Jul 2014 11:21:52 -0400 Original-Received: from mtaout22.012.net.il ([80.179.55.172]:42115) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1X6i4V-00055R-4q for 18006@debbugs.gnu.org; Mon, 14 Jul 2014 11:21:47 -0400 Original-Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0N8P00E00JVSI300@a-mtaout22.012.net.il> for 18006@debbugs.gnu.org; Mon, 14 Jul 2014 18:21:36 +0300 (IDT) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0N8P00EDEK005S80@a-mtaout22.012.net.il>; Mon, 14 Jul 2014 18:21:36 +0300 (IDT) In-reply-to: <53C33C6A.6060601@cs.ucla.edu> X-012-Sender: halo1@inter.net.il X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:91537 Archived-At: > Date: Sun, 13 Jul 2014 19:11:54 -0700 > From: Paul Eggert > CC: 18006@debbugs.gnu.org > > Eli Zaretskii wrote: > > > + return isatty (fd) - 1; > > > Do we need this "-1" part now? It could misfire if isatty does > return 1 some day. > > Sorry, I don't so how it can misfire. isatty returns 1 on success, 0 > on failure; whereas the caller returns 0 on success, -1 on > failure. So subtracting 1 is the right thing to do, no? The MinGW isatty returns a non-zero value for any character device (e.g., the null device), not just for the console. GetConsoleMode fails for anything that isn't a console, so the code will fall through to this line, and return something other than -1. In addition, even when the descriptor _is_ connected to a console, isatty returns a value that is not 1. You already return zero for success, so I think it is better to use an explicit return -1; here. Or did I miss something? > A revised patch, taking your other comments into account, is > attached. It's relative to trunk bzr 117527. Thanks. This goes farther than I intended (I didn't intend to remove _fmode from src/ files), but I guess it's good to get rid of _fmode there as well. It does trigger a couple more comments, though: > === modified file 'src/process.c' > --- src/process.c 2014-07-08 17:13:32 +0000 > +++ src/process.c 2014-07-14 02:02:15 +0000 > @@ -88,6 +88,7 @@ > #include > #endif > > +#include > #include > #include > #include > @@ -3142,6 +3143,8 @@ > continue; > } > > + set_binary_mode (s, O_BINARY); This and other similar changes in process.c are unnecessary: sockets don't need to be switched to binary mode. Moreover, the file descriptor returned by 'sys_socket' (a wrapper for 'socket') on MS-Windows is not used for any actual I/O, it is used as a key for looking up socket handles recorded in a table maintained by w32.c. Of course, if you want to have this for consistency, it cannot do any harm in this case. > /* Open FILE for Emacs use, using open flags OFLAG and mode MODE. > + Use binary I/O on systems that care about text vs binary I/O. > Arrange for subprograms to not inherit the file descriptor. > Prefer a method that is multithread-safe, if available. > Do not fail merely because the open was interrupted by a signal. > @@ -2207,7 +2210,7 @@ > emacs_open (const char *file, int oflags, int mode) > { > int fd; > - oflags |= O_CLOEXEC; > + oflags |= O_BINARY | O_CLOEXEC; > while ((fd = open (file, oflags, mode)) < 0 && errno == EINTR) This is not quite right, as it effectively disallows opening a file in text mode (lread.c, xfaces.c, and termcap.c use that feature). It's probably my fault: I failed to mention that _fmode controls only the _default_ open mode, which can still be overridden by an explicit O_BINARY or O_TEXT flag. So the above addition of O_BINARY should be conditioned on O_TEXT not being set in OFLAGS. Otherwise, the patch looks good to me. Thanks.