Hi Bruno,

Bruno Haible wrote:
>> What do you think of the following two patches?
> 
> I'm not Andreas, but if I may comment on it:

Always appreciated.

> The only things I would do differently:
> 
> 1) If binary_transput is of type bool, the comment
>   /* O_BINARY if binary i/o is desired */
> is no longer correct. In order to reduce the amount of conditional
> expressions, I would preserve the comment and go back to 'int' rather
> than 'bool'.

Oops, yes the comment would be inaccurate. I prefer bool since it is
more clear than an int which can only be  0 or O_BINARY, IMO. But that
is just personal opinion and I assume others would dislike the
conditionals too.

I've attached a fixed patch that sticks to the previous method of 0 or
O_BINARY.

Collin
From b6afbe1cc9a8c893e8dbbea82e889ad72fcc1a1f Mon Sep 17 00:00:00 2001
From: Collin Funk <[email protected]>
Date: Sat, 15 Jun 2024 17:26:57 -0700
Subject: [PATCH v2 2/2] maint: Use gnulib's xbinary-io module.

* bootstrap.conf (gnulib_modules): Depend on xbinary-io.
* m4/setmode.m4: Remove file.
* configure.ac (AC_FUNC_SETMODE_DOS): Remove macro invocation.
* src/common.h (binary_transput): Define unconditionally.
* src/patch.c (main): Style fix.
(get_some_switches): Set binary_transput.
* src/pch.c (open_patch_file): Use xset_binary_mode.
---
 bootstrap.conf |  1 +
 configure.ac   |  1 -
 m4/setmode.m4  | 38 --------------------------------------
 src/common.h   |  6 +-----
 src/patch.c    |  4 +---
 src/pch.c      | 12 ++----------
 6 files changed, 5 insertions(+), 57 deletions(-)
 delete mode 100644 m4/setmode.m4

diff --git a/bootstrap.conf b/bootstrap.conf
index bb89b0f..a749519 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -76,6 +76,7 @@ update-copyright
 utimensat
 verror
 xalloc
+xbinary-io
 xlist
 xmemdup0
 "
diff --git a/configure.ac b/configure.ac
index 8db7e51..468114c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -142,7 +142,6 @@ gl_SIZE_MAX
 gl_FUNC_XATTR
 
 AC_CHECK_FUNCS(geteuid getuid raise sigaction sigprocmask sigsetmask)
-AC_FUNC_SETMODE_DOS
 
 AC_PATH_PROG([ED], [ed], [ed])
 AC_DEFINE_UNQUOTED([EDITOR_PROGRAM], ["$ED"], [Name of editor program.])
diff --git a/m4/setmode.m4 b/m4/setmode.m4
deleted file mode 100644
index 07b529b..0000000
--- a/m4/setmode.m4
+++ /dev/null
@@ -1,38 +0,0 @@
-# Check for setmode, DOS style.
-
-# Copyright (C) 2001-2002, 2011-2012 Free Software Foundation, Inc.
-
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2, or (at your option)
-# any later version.
-
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
-# USA.
-
-AC_DEFUN([AC_FUNC_SETMODE_DOS],
-  [AC_CHECK_HEADERS([fcntl.h unistd.h])
-   AC_CACHE_CHECK([for DOS-style setmode],
-     [ac_cv_func_setmode_dos],
-     [AC_TRY_LINK(
-	[#include <io.h>
-	 #if HAVE_FCNTL_H
-	 # include <fcntl.h>
-	 #endif
-	 #if HAVE_UNISTD_H
-	 # include <unistd.h>
-	 #endif],
-	[int ret = setmode && setmode (1, O_BINARY);],
-	[ac_cv_func_setmode_dos=yes],
-	[ac_cv_func_setmode_dos=no])])
-   if test $ac_cv_func_setmode_dos = yes; then
-     AC_DEFINE([HAVE_SETMODE_DOS], [1],
-       [Define to 1 if you have the DOS-style `setmode' function.])
-   fi])
diff --git a/src/common.h b/src/common.h
index 53c5e32..2c72dfa 100644
--- a/src/common.h
+++ b/src/common.h
@@ -177,11 +177,7 @@ extern int errno;
 
 #include <fcntl.h>
 
-#ifdef HAVE_SETMODE_DOS
-  XTERN int binary_transput;	/* O_BINARY if binary i/o is desired */
-#else
-# define binary_transput 0
-#endif
+XTERN int binary_transput;	/* O_BINARY if binary i/o is desired */
 
 /* Disable the CR stripping heuristic?  */
 XTERN bool no_strip_trailing_cr;
diff --git a/src/patch.c b/src/patch.c
index d5f456c..b37279d 100644
--- a/src/patch.c
+++ b/src/patch.c
@@ -363,7 +363,7 @@ main (int argc, char **argv)
 	if (! skip_rest_of_patch && ! outfile)
 	  {
 	    init_output (&outstate);
-	    outstate.ofp = fdopen(outfd, binary_transput ? "wb" : "w");
+	    outstate.ofp = fdopen (outfd, binary_transput ? "wb" : "w");
 	    if (! outstate.ofp)
 	      pfatal ("%s", TMPOUTNAME);
 	    /* outstate.ofp now owns the file descriptor */
@@ -1033,9 +1033,7 @@ get_some_switches (void)
 		break;
 	    case CHAR_MAX + 3:
 		no_strip_trailing_cr = true;
-#if HAVE_SETMODE_DOS
 		binary_transput = O_BINARY;
-#endif
 		break;
 	    case CHAR_MAX + 4:
 		usage (stdout, 0);
diff --git a/src/pch.c b/src/pch.c
index fd9c480..4f20dcd 100644
--- a/src/pch.c
+++ b/src/pch.c
@@ -29,12 +29,10 @@
 #undef XTERN
 #define XTERN
 #include <pch.h>
-#if HAVE_SETMODE_DOS
-# include <io.h>
-#endif
 #include <safe.h>
 #include <alloca.h>
 #include "execute.h"
+#include "xbinary-io.h"
 
 #define INITHUNKMAX 125			/* initial dynamic allocation size */
 
@@ -125,14 +123,8 @@ open_patch_file (char const *filename)
 	if (!pfp)
 	  pfatal ("Can't open patch file %s", quotearg (filename));
       }
-#if HAVE_SETMODE_DOS
     if (binary_transput)
-      {
-	if (isatty (fileno (pfp)))
-	  fatal ("cannot read binary data from tty on this platform");
-	setmode (fileno (pfp), O_BINARY);
-      }
-#endif
+      xset_binary_mode (fileno (pfp), O_BINARY);
     if (fstat (fileno (pfp), &st) != 0)
       pfatal ("fstat");
     if (S_ISREG (st.st_mode) && (pos = file_tell (pfp)) != -1)
-- 
2.45.2

Reply via email to