[PATCH] Use O_BINARY to detect whether to pass "rb" to popen

Discussion:

(too old to reply)

Filipe Brandenburger

2015-05-21 05:15:46 UTC

Get rid of freadonly_mode and fwriteonly_mode which were set from a
define set by a configure AC_RUN_IFELSE check.

Instead, use the previously already defined FOPEN_READ_BINARY and
FOPEN_WRITE_BINARY which are set based on O_BINARY from fcntl.h, which
will be defined to be non-zero on systems where there is a difference
between text and binary files.

Remove the now unnecessary configure check for whether popen works with
binary mode. That particular check required running a program built by
configure, which made it hard or impossible to cross-compile sharutils
without patching the sources.

Tested that it still builds and that the testsuite of `make check`
passes. Checked that `make distcheck` works as expected.

Tested it on a Linux/glibc system and on Windows and cygwin (the closest
to a system where there is a difference between text and binary files,
though cygwin doesn't really seem to do anything with "b" on popen.)

Tested that cross-compiling works on the pristine sources from the dist
tarball without any modifications to the source tree.

Signed-off-by: Filipe Brandenburger <***@google.com>
---
ChangeLog | 7 +++++++
configure.ac.git | 6 ------
src/local.h.git | 10 ----------
src/shar.c | 20 ++++++++++----------
4 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 043c5abfa5ce..1784941ec6d7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2015-05-20 Filipe Brandenburger <***@google.com>
+
+ * src/shar.c: Use FOPEN_{READ,WRITE}_BINARY constants derived from
+ O_BINARY instead of f{read,write}only_binary probed by autoconf.
+ * src/local.h.git: Drop the no longer needed f{read,write}only_mode.
+ * configure.ac.git: Drop the check for popen with "rb" mode.
+
2015-01-06 Eric Blake <***@redhat.com>

build: avoid error message failure on 64-bit hosts
diff --git a/configure.ac.git b/configure.ac.git
index 6ab67d9ed2e2..a669db093e67 100644
--- a/configure.ac.git
+++ b/configure.ac.git
@@ -94,12 +94,6 @@ AM_WITH_DMALLOC
CATALOGS="$new_CATALOGS"
fi]

-AC_RUN_IFELSE(
- [AC_LANG_PROGRAM([], [dnl
- FILE * fp = popen ("date", "rb");
- exit (fp == NULL);])],
- [AC_DEFINE([BINARY_MODE_POPEN], [1], [define if popen supports "rb" mode])]
-)
# INVOKE_LIBOPTS_MACROS
INVOKE_LIBOPTS_MACROS

diff --git a/src/local.h.git b/src/local.h.git
index 8f234cbe5d6e..f778c09f773d 100644
--- a/src/local.h.git
+++ b/src/local.h.git
@@ -22,14 +22,4 @@

INCLUDE-LIST

-#ifdef SHAR_C
-# ifdef BINARY_MODE_POPEN
-static char const freadonly_mode[] = "rb";
-static char const fwriteonly_mode[] = "wb";
-# else
-static char const freadonly_mode[] = "r";
-static char const fwriteonly_mode[] = "w";
-# endif
-#endif
-
#endif /* SHARUTILS_LOCAL_H */
diff --git a/src/shar.c b/src/shar.c
index 5507b3b71962..54bb28d9efd3 100644
--- a/src/shar.c
+++ b/src/shar.c
@@ -1170,7 +1170,7 @@ file_needs_encoding (char const * fname)
average file size, even reading the whole file (if it is text) would
usually be faster than invoking 'file'. */

- infp = fopen (fname, freadonly_mode);
+ infp = fopen (fname, FOPEN_READ_BINARY);

if (infp == NULL)
{
@@ -1265,11 +1265,11 @@ encode_file_to_pipe (
{
sprintf (cmdline, cmpr_state->cmpr_cmd_fmt,
cmpr_state->cmpr_level, q_local_name);
- in_fp = popen (cmdline, freadonly_mode);
+ in_fp = popen (cmdline, FOPEN_READ_BINARY);
}
else
{
- in_fp = fopen (local_name, freadonly_mode);
+ in_fp = fopen (local_name, FOPEN_READ_BINARY);
open_fmt = "fopen";
open_txt = local_name;
}
@@ -1277,7 +1277,7 @@ encode_file_to_pipe (
if (in_fp == NULL)
fserr (SHAR_EXIT_FAILED, open_fmt, open_txt);

- out_fp = fdopen (out_fd, fwriteonly_mode);
+ out_fp = fdopen (out_fd, FOPEN_WRITE_BINARY);

fprintf (out_fp, mode_fmt_z, restore_name);

@@ -1321,7 +1321,7 @@ open_encoded_file (char const * local_name, char const * q_local_name,
close (pipex[1]);

{
- FILE * fp = fdopen (pipex[0], freadonly_mode);
+ FILE * fp = fdopen (pipex[0], FOPEN_READ_BINARY);
if (fp == NULL)
fserr (SHAR_EXIT_FAILED, "fdopen", _("pipe fd"));
return fp;
@@ -1402,7 +1402,7 @@ open_encoded_file (char const * local_name,
sprintf (p, uu_cmd_fmt, restore_name);
}

- /* Don't use freadonly_mode because it might be "rb", while we need
+ /* Don't use FOPEN_READ_BINARY because it might be "rb", while we need
text-mode read here, because we will be reading pure text from
uuencode, and we want to drop any CR characters from the CRLF
line endings, when we write the result into the shar. */
@@ -1441,7 +1441,7 @@ open_shar_input (
*file_type_p = _("text");
*file_type_remote_p = SM_type_text;

- infp = fopen (local_name, freadonly_mode);
+ infp = fopen (local_name, FOPEN_READ_BINARY);
if (infp == NULL)
fserr (SHAR_EXIT_FAILED, "fopen", local_name);
*pipe_p = 0;
@@ -1822,7 +1822,7 @@ finish_sharing_file (const char * lname, const char * lname_q,
echo_status ("test $? -ne 0", SM_restore_failed, NULL, rname, 0);

if ( ! HAVE_OPT(NO_MD5_DIGEST)
- && (fp = fopen (lname, freadonly_mode)) != NULL
+ && (fp = fopen (lname, FOPEN_READ_BINARY)) != NULL
&& md5_stream (fp, md5buffer) == 0)
{
/* Validate the transferred file using 'md5sum' command. */
@@ -2050,7 +2050,7 @@ open_output (void)
if (output_filename == NULL)
parse_output_base_name (OPT_ARG(OUTPUT_PREFIX));
sprintf (output_filename, OPT_ARG(OUTPUT_PREFIX), ++part_number);
- output = fopen (output_filename, fwriteonly_mode);
+ output = fopen (output_filename, FOPEN_WRITE_BINARY);

if (output == NULL)
fserr (SHAR_EXIT_FAILED, _("Opening"), output_filename);
@@ -2152,7 +2152,7 @@ configure_shar (int * argc_p, char *** argv_p)
* Redirect stderr to /dev/null in that case so that
* the results are not cluttered with chatter.
*/
- FILE * fp = freopen ("/dev/null", fwriteonly_mode, stderr);
+ FILE * fp = freopen ("/dev/null", FOPEN_WRITE_BINARY, stderr);
if (fp != stderr)
error (SHAR_EXIT_FAILED, errno,
_("reopening stderr to /dev/null"));

--
2.2.0.rc0.207.ga3a616c

Eli Zaretskii

2015-05-21 16:19:25 UTC