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