GNU bug report logs - #28083
Two patches for grep on Windows, fixing directory recursion and leaking color

Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.

Package: grep; Reported by: Stephan Lavavej <stl@HIDDEN>; dated Sun, 13 Aug 2017 22:55:02 UTC; Maintainer for grep is bug-grep@HIDDEN.

Message received at submit <at> debbugs.gnu.org:


Received: (at submit) by debbugs.gnu.org; 13 Aug 2017 22:54:15 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Aug 13 18:54:15 2017
Received: from localhost ([127.0.0.1]:33253 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1dh1lv-0001U3-77
	for submit <at> debbugs.gnu.org; Sun, 13 Aug 2017 18:54:15 -0400
Received: from eggs.gnu.org ([208.118.235.92]:57881)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <stl@HIDDEN>) id 1dh1lt-0001Tx-RT
 for submit <at> debbugs.gnu.org; Sun, 13 Aug 2017 18:54:14 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
 (envelope-from <stl@HIDDEN>) id 1dh1lm-0006kX-Rm
 for submit <at> debbugs.gnu.org; Sun, 13 Aug 2017 18:54:08 -0400
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org
X-Spam-Level: 
X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50,T_DKIM_INVALID
 autolearn=disabled version=3.3.2
Received: from lists.gnu.org ([2001:4830:134:3::11]:44904)
 by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)
 (Exim 4.71) (envelope-from <stl@HIDDEN>) id 1dh1lm-0006kP-Nt
 for submit <at> debbugs.gnu.org; Sun, 13 Aug 2017 18:54:06 -0400
Received: from eggs.gnu.org ([2001:4830:134:3::10]:48850)
 by lists.gnu.org with esmtp (Exim 4.71)
 (envelope-from <stl@HIDDEN>) id 1dh1lk-0007ZI-Pq
 for bug-grep@HIDDEN; Sun, 13 Aug 2017 18:54:06 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
 (envelope-from <stl@HIDDEN>) id 1dh1lg-0006h0-RF
 for bug-grep@HIDDEN; Sun, 13 Aug 2017 18:54:04 -0400
Received: from mail-it0-x244.google.com ([2607:f8b0:4001:c0b::244]:35781)
 by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)
 (Exim 4.71) (envelope-from <stl@HIDDEN>) id 1dh1lg-0006g5-Kf
 for bug-grep@HIDDEN; Sun, 13 Aug 2017 18:54:00 -0400
Received: by mail-it0-x244.google.com with SMTP id 76so7007153ith.2
 for <bug-grep@HIDDEN>; Sun, 13 Aug 2017 15:53:58 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=nuwen-net.20150623.gappssmtp.com; s=20150623;
 h=mime-version:from:date:message-id:subject:to
 :content-transfer-encoding;
 bh=0ssIVd6r2w3+XJFaPbg7tdgRLkX+GbrnakkrC0pr1uk=;
 b=knuctDzlN4t2viruIDyqgVzhB+lstSSGj/AZJ8rSRXQnC1ldPB9NuHDK5szeE7Lmlu
 5eeTmPjhoAmcLYSrPEwxH93D8++WMUmkW51ZQd00LhOLAVAle+7JeixbpEHEAjWpcg4+
 lqWiFohzGJrc/F0VGeySHFtCQ4zVGBVQmiXaDlHyKg06DDsatha7rl6jtARMK8TVf4lc
 RddI8lgHHDf8JVfF0+WKfyliI0jLisL0cA8K1kIghpJb4C6KccZm4GQjTnKCOdb95EwY
 tnYeDoPNIre6YS72qKIauAWWNCCB0hWsSwX0VcN08TwvzG1HNn40Rc4ggtAQNSnfBm5Q
 DNnQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:from:date:message-id:subject:to
 :content-transfer-encoding;
 bh=0ssIVd6r2w3+XJFaPbg7tdgRLkX+GbrnakkrC0pr1uk=;
 b=M2b3pkfQZSEMPli6EbR5aHbPpvJhT1SE0zHGgsAB/CkS9AdK05VLKEjp2iR9Fdfmhc
 V+hfurfTjbwpAxxSDtyw2q20932d9dJMxVTbd4N8QeKs7sae8zv7zZKqKOpgt/Bdt9fo
 ej7LyxLeIaZR7eSifLIvrY4z+5hAe5GMfOhejkFzOT37Re/vPfReLWAKiLAu/8ajjpoy
 Qfpwb9tvJpWH3n4N8vv0sCk0Dctu/+xUPwexnAtSMoafNTLrdcm7GjEvgSID2C0iP46g
 ToEkSx/WD8CB6KyV/1iFVNNgxc+L5+KczKdDHhiKUCafIGlVnbSrgdOEm5i8VF5BGBzl
 U/FQ==
X-Gm-Message-State: AHYfb5iuwModpw0uNeO8L4LtucSKzpE8RtHZPox05DjWSMh/U7Yu/xPl
 sn6tzQh7sTyuN8AT7gEVJsRXv8GJ+xjK69o=
X-Received: by 10.36.135.202 with SMTP id f193mr4125100ite.48.1502664837559;
 Sun, 13 Aug 2017 15:53:57 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.2.88.69 with HTTP; Sun, 13 Aug 2017 15:53:57 -0700 (PDT)
X-Originating-IP: [50.35.79.58]
From: Stephan Lavavej <stl@HIDDEN>
Date: Sun, 13 Aug 2017 15:53:57 -0700
Message-ID: <CACHw8XKa-xFpUXJ407-5P2i1DEdTp4hEwvZBRMO9N6y9Ui45rg@HIDDEN>
Subject: Two patches for grep on Windows, fixing directory recursion and
 leaking color
To: bug-grep@HIDDEN
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-detected-operating-system: by eggs.gnu.org: Genre and OS details not
 recognized.
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x
X-Received-From: 2001:4830:134:3::11
X-Spam-Score: -4.0 (----)
X-Debbugs-Envelope-To: submit
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -4.0 (----)

Hi,

I have two patches that improve grep's support for Windows. The first
patch is a one-liner that fixes directory recursion, while the second
patch fixes "leaking color" when grep is printing colorized output and
is terminated prematurely by Ctrl+C.

First patch: gnulib recently gained a module, windows-stat-inodes,
that fixes directory recursion on Windows. No changes to grep's C
sources are required - grep simply needs to request the module during
configuration. Here's how:

###

Add windows-stat-inodes to bootstrap.conf, fixing directory recursion
on Windows.

diff --git a/bootstrap.conf b/bootstrap.conf
index 1c50974..73f1573 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -93,6 +93,7 @@ wchar
 wcrtomb
 wctob
 wctype-h
+windows-stat-inodes
 xalloc
 xbinary-io
 xstrtoimax

###

Here's the gnulib commit that added the windows-stat-inodes module:
https://git.savannah.gnu.org/gitweb/?p=3Dgnulib.git;a=3Dcommit;h=3D8123b614=
e616eaa951d842f10730ba7b914f75b3

When grep requests this module, its configure script will gain the
behavior that was implemented in windows-stat-inodes.m4. This detects
mingw and sets WINDOWS_STAT_INODES=3D1. All other platforms are
unaffected, setting WINDOWS_STAT_INODES=3D0 (which is what's happening
in the absence of this patch).

(Credit: Thanks to P=C3=A4r Bj=C3=B6rklund who diagnosed the problem as
involving inodes, which led me to discover the gnulib module, and
thanks to V=C3=A1clav Haisman who provided the bootstrap.conf patch in
https://github.com/StephanTLavavej/mingw-distro/issues/41 .)


Second patch: grep implements colorized output on Windows with
SetConsoleTextAttribute(). However, if grep is terminated prematurely
by Ctrl+C when it's in the middle of printing colorized output, the
Windows console won't reset the color. This results in "leaking
color", where the prompt is displayed in whatever color that grep was
using for highlighting. The console's color can be restored by running
"color 07" (or whatever the normal color is), but the problem can be
avoided completely.

Fixing this problem is fairly simple. The WinAPI provides
SetConsoleCtrlHandler(), allowing a program to intercept Ctrl+C (and
Ctrl+Break, which is identically affected). On Windows, Ctrl+C works
by injecting an additional thread into the process. Note that my patch
isn't introducing this thread - it's always injected by Ctrl+C, which
uses a default handler to terminate the process. My patch simply adds
an additional handler to be called. So to avoid leaking color, we need
to:

1. Initialize a critical section, which cannot fail on Vista and
above. (If anyone cares about XP/2003, it can fail under conditions of
extremely low memory, which isn't a concern in practice.)

2. Whenever we're about to colorize the output, enter the critical
section and check a bool (initially true) saying whether we're allowed
to colorize. If so, call SetConsoleTextAttribute() while still inside
that critical section. If not, do nothing.

3. During initialization, use SetConsoleCtrlHandler() to add a small
function that will be called when Ctrl+C (or Ctrl+Break) is pressed.
This will enter the critical section, restore the console's text
color, and set the bool to false.

The critical section is necessary because when Windows injects the
thread for Ctrl+C, the main thread can be busy toggling the color on
and off. By resetting the text color and setting the bool to false,
the main thread is prevented from re-colorizing the console.

(The main thread also calls SetConsoleTextAttribute() at the end of
colorized regions. This doesn't need to be guarded by the critical
section, since repeatedly restoring the console's color is perfectly
fine.)

I've been using this patch for many years (going back to grep 2.10 in
March 2012 at least, when grep lacked support for color on Windows and
I added it locally). In this patch, I've tried to follow the
surrounding coding conventions to the best of my ability. As the
changes are limited to colorize-w32.c, no other platforms are
affected.

###

On Windows, restore the console's color when Ctrl+C is pressed.

diff --git a/lib/colorize-w32.c b/lib/colorize-w32.c
index 1073018..0ae014b 100644
--- a/lib/colorize-w32.c
+++ b/lib/colorize-w32.c
@@ -32,6 +32,27 @@

 static HANDLE hstdout =3D INVALID_HANDLE_VALUE;
 static SHORT norm_attr;
+static CRITICAL_SECTION color_lock;
+static BOOL color_bool =3D TRUE;
+
+/* After Ctrl+C or Ctrl+Break,
+   restore the normal text attribute used by the console.  */
+static BOOL WINAPI
+w32_color_handler (DWORD ctrl_type)
+{
+  if (ctrl_type =3D=3D CTRL_C_EVENT
+      || ctrl_type =3D=3D CTRL_BREAK_EVENT)
+    {
+      EnterCriticalSection (&color_lock);
+      if (hstdout !=3D INVALID_HANDLE_VALUE)
+        {
+          SetConsoleTextAttribute (hstdout, norm_attr);
+        }
+      color_bool =3D FALSE;
+      LeaveCriticalSection (&color_lock);
+    }
+  return FALSE;
+}

 /* Initialize the normal text attribute used by the console.  */
 void
@@ -45,6 +66,9 @@ init_colorize (void)
      norm_attr =3D csbi.wAttributes;
   else
     hstdout =3D INVALID_HANDLE_VALUE;
+
+  InitializeCriticalSectionAndSpinCount (&color_lock, 4000);
+  SetConsoleCtrlHandler (&w32_color_handler, TRUE);
 }

 /* Return non-zero if we should highlight matches in output.  */
@@ -164,7 +188,12 @@ print_start_colorize (char const *sgr_start, char
const *sgr_seq)
   if (hstdout !=3D INVALID_HANDLE_VALUE)
     {
       SHORT attr =3D w32_sgr2attr (sgr_seq);
-      SetConsoleTextAttribute (hstdout, attr);
+      EnterCriticalSection (&color_lock);
+      if (color_bool)
+        {
+          SetConsoleTextAttribute (hstdout, attr);
+        }
+      LeaveCriticalSection (&color_lock);
     }
   else
     printf (sgr_start, sgr_seq);

###

This patch is also available at:
https://github.com/StephanTLavavej/mingw-distro/blob/fbe3e6bcb952c8b48af0eb=
f4c0e13c08ffb8aea4/grep-lock.patch

Thank you very much for your continued maintenance of grep.

STL




Acknowledgement sent to Stephan Lavavej <stl@HIDDEN>:
New bug report received and forwarded. Copy sent to bug-grep@HIDDEN. Full text available.
Report forwarded to bug-grep@HIDDEN:
bug#28083; Package grep. Full text available.
Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.
Last modified: Sun, 13 Aug 2017 23:00:02 UTC

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997 nCipher Corporation Ltd, 1994-97 Ian Jackson.