Subject: bug#51860: closed (Re: bug#51860: [PATCH] Reinstate Binary file matches to stdout)

From: [email-masked] (GNU bug Tracking System)
Date: Sun, 21 Nov 2021 07:00:02 +0000

Your bug report

#51860: [PATCH] Reinstate Binary file matches to stdout

which was filed against the grep package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
51860: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=51860
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#51860: [PATCH] Reinstate Binary file matches to stdout
From: Paul Eggert <[email-masked]>
To: Duncan Roe <[email-masked]>
Cc: [email-masked]
Date: Sat, 20 Nov 2021 22:59:43 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

On 11/20/21 17:38, Duncan Roe wrote:

> This was *documented* *behaviour*.

No, it wasn't documented whether the "Binary file matches" message is 
sent to stdout or to stderr. But now that you mention it, this should be 
documented. First patch attached, and installed into the development 
version.

> I'd further suggest biaary matches output to stderr by the new option be not
> affected by the setting of `-s`, only by `-I`. This reinstates orthogonality of
> -s and -I which 271793f0 broke.

Hmm, you have a point that -s is documented to "Suppress error messages 
about nonexistent or unreadable files" and it's plausible to say that 
the "binary file matches" diagnostic, although it is about a binary file 
that 'grep' has some trouble reading, is not about a file that is 
"unreadable". So, let's fix the code to match that part of the spec 
again. Second patch attached and installed.

> Since you have provided me with a workaround, I have no further interset in this
> bug.

OK, closing the bug report. Thanks for reporting the issues.

From 56762bfda567ac2f96ab2f88e8117b5602bccddb Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 20 Nov 2021 22:51:26 -0800
Subject: [PATCH 1/2] doc: "binary file matches" -> stderr [Bug#51860]

---
 doc/grep.in.1 | 2 +-
 doc/grep.texi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/grep.in.1 b/doc/grep.in.1
index 208cb76..123b8d6 100644
--- a/doc/grep.in.1
+++ b/doc/grep.in.1
@@ -520,7 +520,7 @@ and suppresses output lines that contain improperly encoded data.
 When some output is suppressed,
 .B grep
 follows any output
-with a one-line message saying that a binary file matches.
+with a message to standard error saying that a binary file matches.
 .IP
 If
 .I TYPE
diff --git a/doc/grep.texi b/doc/grep.texi
index c3c4bbf..b1da5cb 100644
--- a/doc/grep.texi
+++ b/doc/grep.texi
@@ -615,7 +615,7 @@ By default, @var{type} is @samp{binary}, and @command{grep}
 suppresses output after null input binary data is discovered,
 and suppresses output lines that contain improperly encoded data.
 When some output is suppressed, @command{grep} follows any output
-with a one-line message saying that a binary file matches.
+with a message to standard error saying that a binary file matches.
 
 If @var{type} is @samp{without-match},
 when @command{grep} discovers null input binary data
-- 
2.32.0

From af79b17356f2edeca2908c14d922a24f659d4a96 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 20 Nov 2021 22:53:55 -0800
Subject: [PATCH 2/2] =?UTF-8?q?grep:=20-s=20does=20not=20suppress=20?=
 =?UTF-8?q?=E2=80=9Cbinary=20file=20matches=E2=80=9D?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/grep.c (grep): Implement this.
* tests/binary-file-matches: Add regression test.
---
 NEWS                      | 5 +++++
 src/grep.c                | 2 +-
 tests/binary-file-matches | 8 +++++---
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 2f63071..5015580 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,11 @@ GNU grep NEWS                                    -*- outline -*-
   release 2.5.3 (2007), now warn that they are obsolescent and should
   be replaced by grep -E and grep -F.
 
+** Bug fixes
+
+  The -s option no longer suppresses "binary file matches" messages.
+  [Bug#51860 introduced in grep 3.5]
+
 
 * Noteworthy changes in release 3.7 (2021-08-14) [stable]
 
diff --git a/src/grep.c b/src/grep.c
index a55194c..19dff43 100644
--- a/src/grep.c
+++ b/src/grep.c
@@ -1646,7 +1646,7 @@ grep (int fd, struct stat const *st, bool *ineof)
  finish_grep:
   done_on_match = done_on_match_0;
   out_quiet = out_quiet_0;
-  if (binary_files == BINARY_BINARY_FILES && ! (out_quiet | suppress_errors)
+  if (binary_files == BINARY_BINARY_FILES && !out_quiet
       && (encoding_error_output
           || (0 <= nlines_first_null && nlines_first_null < nlines)))
     error (0, 0, _("%s: binary file matches"), input_filename ());
diff --git a/tests/binary-file-matches b/tests/binary-file-matches
index 7fc4a11..8fea071 100755
--- a/tests/binary-file-matches
+++ b/tests/binary-file-matches
@@ -14,8 +14,10 @@ fail=0
 echo "grep: (standard input): binary file matches" > exp \
   || framework_failure_
 
-printf 'a\0' | grep a > out 2> err || fail=1
-compare /dev/null out || fail=1
-compare exp err || fail=1
+for option in '' -s; do
+  printf 'a\0' | grep $option a > out 2> err || fail=1
+  compare /dev/null out || fail=1
+  compare exp err || fail=1
+done
 
 Exit $fail
-- 
2.32.0

Subject: [PATCH] Reinstate Binary file matches to stdout
From: Duncan Roe <[email-masked]>
To: [email-masked]
Cc: Duncan Roe <[email-masked]>
Date: Mon, 15 Nov 2021 17:06:29 +1100
List-Id: <debbugs-submit.debbugs.gnu.org>

Revert commit 271793f0.

* NEWS: Mention this.
* doc/grep.texi: remove 1 line added in 271793f0.
* src/grep.c (grep): Send traditional "Binary file FOO matches"
to stdout again.
* tests/surrogate-pair: test for traditional behaviour (test updated
since 271793f0).
* tests/null-byte: Reverted (non-conflicting changes since 271793f0).
* tests/encoding-error, tests/invalid-multibyte-infloop:
* tests/pcre-count, tests/symlink, tests/unibyte-binary:
Reverted (unchanged since 271793f0).
* tests/binary-file-matches: test for traditional behavior (new test).

Signed-off-by: Duncan Roe <[email-masked]>
---
 NEWS                            |  6 ++++++
 doc/grep.texi                   |  3 +--
 src/grep.c                      |  8 ++++++--
 tests/binary-file-matches       |  6 +++---
 tests/encoding-error            |  5 +++--
 tests/invalid-multibyte-infloop |  4 +++-
 tests/null-byte                 |  2 +-
 tests/pcre-count                |  5 +++--
 tests/surrogate-pair            | 12 ++++++++----
 tests/symlink                   |  6 +++++-
 tests/unibyte-binary            |  2 +-
 11 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/NEWS b/NEWS
index 2f63071..0173187 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,12 @@ GNU grep NEWS                                    -*- outline -*-
 
 ** Changes in behavior
 
+  The message that a binary file matches is again sent to standard output
+  with the wording as it was before release 3.5. The -I option is available
+  to suppress this message if so desired. The behavior in grep 3.5
+  through 3.7 prevented a user seeing binary matches when using
+  grep -s to suppress error messages.
+
   The -P option is now based on PCRE2 instead of the older PCRE,
   thanks to code contributed by Carlo Arenas.
 
diff --git a/doc/grep.texi b/doc/grep.texi
index c3c4bbf..2482fa0 100644
--- a/doc/grep.texi
+++ b/doc/grep.texi
@@ -1819,8 +1819,7 @@ to output lines even from files that appear to be binary, use the
 @option{-a} or @samp{--binary-files=text} option.
 To eliminate the
 ``Binary file matches'' messages, use the @option{-I} or
-@samp{--binary-files=without-match} option,
-or the @option{-s} or @option{--no-messages} option.
+@samp{--binary-files=without-match} option.
 
 @item
 Why doesn't @samp{grep -lv} print non-matching file names?
diff --git a/src/grep.c b/src/grep.c
index a55194c..0e60023 100644
--- a/src/grep.c
+++ b/src/grep.c
@@ -1646,10 +1646,14 @@ grep (int fd, struct stat const *st, bool *ineof)
  finish_grep:
   done_on_match = done_on_match_0;
   out_quiet = out_quiet_0;
-  if (binary_files == BINARY_BINARY_FILES && ! (out_quiet | suppress_errors)
+  if (binary_files == BINARY_BINARY_FILES && !out_quiet
       && (encoding_error_output
           || (0 <= nlines_first_null && nlines_first_null < nlines)))
-    error (0, 0, _("%s: binary file matches"), input_filename ());
+    {
+      printf_errno (_("Binary file %s matches\n"), input_filename ());
+      if (line_buffered)
+        fflush_errno ();
+    }
   return nlines;
 }
 
diff --git a/tests/binary-file-matches b/tests/binary-file-matches
index 7fc4a11..dc4c3e7 100755
--- a/tests/binary-file-matches
+++ b/tests/binary-file-matches
@@ -11,11 +11,11 @@
 
 fail=0
 
-echo "grep: (standard input): binary file matches" > exp \
+echo "Binary file (standard input) matches" > exp \
   || framework_failure_
 
 printf 'a\0' | grep a > out 2> err || fail=1
-compare /dev/null out || fail=1
-compare exp err || fail=1
+compare /dev/null err || fail=1
+compare exp out || fail=1
 
 Exit $fail
diff --git a/tests/encoding-error b/tests/encoding-error
index 2ec71c9..9fb496c 100755
--- a/tests/encoding-error
+++ b/tests/encoding-error
@@ -25,7 +25,8 @@ grep '^A' in >out || fail=1
 compare a out || fail=1
 
 grep '^P' in >out || fail=1
-compare /dev/null out || fail=1
+printf 'Binary file in matches\n' >exp || framework_failure_
+compare exp out || fail=1
 
 grep -I '^P' in >out 2>err || fail=1
 compare /dev/null out || fail=1
@@ -38,7 +39,7 @@ returns_ 1 grep '^X' in >out || fail=1
 compare /dev/null out || fail=1
 
 grep . in >out || fail=1
-cat a j >exp || framework_failure_
+(cat a j && printf 'Binary file in matches\n') >exp || framework_failure_
 compare exp out || fail=1
 
 grep -I . in >out 2>err || fail=1
diff --git a/tests/invalid-multibyte-infloop b/tests/invalid-multibyte-infloop
index b4ad14b..5b3bdfc 100755
--- a/tests/invalid-multibyte-infloop
+++ b/tests/invalid-multibyte-infloop
@@ -24,10 +24,12 @@ else
   test $status -eq 2
 fi || fail=1
 
+echo 'Binary file input matches' >binary-file-matches
+
 LC_ALL=en_US.UTF-8 timeout 10 grep -F $(encode A) input > out
 status=$?
 if test $status -eq 0; then
-  compare /dev/null out
+  compare binary-file-matches out
 elif test $status -eq 1; then
   compare_dev_null_ /dev/null out
 else
diff --git a/tests/null-byte b/tests/null-byte
index d86c249..9354aaf 100755
--- a/tests/null-byte
+++ b/tests/null-byte
@@ -56,7 +56,7 @@ echo xxx >exp || framework_failure_
 grep xxx in >out || fail=1
 compare exp out || fail=1
 
-printf 'xxx\n' > exp || framework_failure_
+printf '%s\n' xxx 'Binary file in matches' > exp || framework_failure_
 grep -E 'xxx|z' in >out || fail=1
 compare exp out || fail=1
 
diff --git a/tests/pcre-count b/tests/pcre-count
index 9eda54b..656780e 100755
--- a/tests/pcre-count
+++ b/tests/pcre-count
@@ -17,9 +17,10 @@ printf 'a\n%032768d\nb\0\n%032768d\na\n' 0 0 > in || framework_failure_
 
 # grep will discover that the input is a binary file sooner if the
 # page size is larger, so allow for either possible output.
-printf 'a\n' >exp1a || framework_failure_
+printf 'a\nBinary file in matches\n' >exp1a || framework_failure_
+printf 'Binary file in matches\n' >exp1b || framework_failure_
 LC_ALL=C grep -P 'a' in >out || fail=1
-compare exp1a out || compare /dev/null out || fail=1
+compare exp1a out || compare exp1b out || fail=1
 
 printf '2\n' >exp2 || framework_failure_
 LC_ALL=C grep -Pc 'a' in >out || fail=1
diff --git a/tests/surrogate-pair b/tests/surrogate-pair
index a91fa36..7f8373a 100755
--- a/tests/surrogate-pair
+++ b/tests/surrogate-pair
@@ -26,6 +26,10 @@ fail=0
 s_pair=$(printf '\360\220\220\205')
 printf '%s\n' "$s_pair" > in || framework_failure_
 
+# On platforms where wchar_t is only 16 bits, wchar_t cannot represent
+# the character encoded in 'in', so accept that behavior too.
+printf 'Binary file in matches\n' > out16 || framework_failure_
+
 LC_ALL=en_US.UTF-8
 export LC_ALL
 
@@ -41,12 +45,12 @@ grep . in > out 2> err || fail=1
 # On platforms where wchar_t is only 16 bits, wchar_t cannot represent
 # the character encoded in 'in'.
 
-# On such old systems the above prints nothing on stdout and a diagnostic
-# on stderr.  In that case, return early; otherwise, the following tests
+# On such old systems the above prints  diagnostic on stdout.
+# In that case, return early; otherwise, the following tests
 # would all fail.
 io_pair=$(cat out):$(cat err)
 case $io_pair in
-  :'grep: in: binary file matches') Exit $fail;;
+  'Binary file in matches:') Exit $fail;;
   $s_pair:) ;;
   *) fail_ "unexpected output: $io_pair"; fail=1;;
 esac
@@ -54,7 +58,7 @@ esac
 # Also test whether a surrogate-pair in the search string works.
 for opt in '' -i -E -F -iE -iF; do
   grep --file=in $opt in > out 2>&1 || fail=1
-  compare out in || fail=1
+  compare out in || compare out out16 || fail=1
 done
 
 Exit $fail
diff --git a/tests/symlink b/tests/symlink
index b580ce7..427dfed 100755
--- a/tests/symlink
+++ b/tests/symlink
@@ -58,7 +58,11 @@ do
 
     printf "$exp" >exp || framework_failure_
 
-    LC_ALL=C sort grepout >out || fail=1
+    LC_ALL=C sort grepout >out-t || fail=1
+
+    # Ignore "Binary file d matches" on systems for which
+    # reading from a directory actually succeeds.
+    grep -v Binary out-t > out; case $? in 0|1) ;; *) fail=1;; esac
 
     compare exp out || fail=1
   done
diff --git a/tests/unibyte-binary b/tests/unibyte-binary
index f76276f..7e8f2aa 100755
--- a/tests/unibyte-binary
+++ b/tests/unibyte-binary
@@ -22,7 +22,7 @@ require_unibyte_locale
 fail=0
 
 printf 'a\n\200\nb\n' >in || framework_failure_
-printf 'a\n' >exp || framework_failure_
+printf 'a\nBinary file in matches\n' >exp || framework_failure_
 grep . in >out || fail=1
 
 # In some unibyte locales, \200 is an encoding error;
-- 
2.33.1