--- Begin Message ---
|
Subject: |
Two patches for grep on Windows, fixing directory recursion and leaking color |
|
Date: |
Sun, 13 Aug 2017 15:53:57 -0700 |
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=gnulib.git;a=commit;h=8123b614e616eaa951d842f10730ba7b914f75b3
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=1. All other platforms are
unaffected, setting WINDOWS_STAT_INODES=0 (which is what's happening
in the absence of this patch).
(Credit: Thanks to Pär Björklund who diagnosed the problem as
involving inodes, which led me to discover the gnulib module, and
thanks to Václav 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 = INVALID_HANDLE_VALUE;
static SHORT norm_attr;
+static CRITICAL_SECTION color_lock;
+static BOOL color_bool = 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 == CTRL_C_EVENT
+ || ctrl_type == CTRL_BREAK_EVENT)
+ {
+ EnterCriticalSection (&color_lock);
+ if (hstdout != INVALID_HANDLE_VALUE)
+ {
+ SetConsoleTextAttribute (hstdout, norm_attr);
+ }
+ color_bool = 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 = csbi.wAttributes;
else
hstdout = 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 != INVALID_HANDLE_VALUE)
{
SHORT attr = 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/fbe3e6bcb952c8b48af0ebf4c0e13c08ffb8aea4/grep-lock.patch
Thank you very much for your continued maintenance of grep.
STL
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: Two patches for grep on Windows, fixing directory recursion and leaking color |
|
Date: |
Wed, 17 Nov 2021 12:34:21 -0800 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.1 |
On 11/17/21 04:37, Stephan T. Lavavej wrote:
I found that the copyright assignment process was more burdensome than I
expected (in particular, the request for employer signoff), so I abandoned
the attempt.
Oh well. Thanks for letting us know. Closing the bug report.
--- End Message ---