Re: MS-Windows build of Grep [2/4]

[Paul Eggert]

On 12/28/11 09:25, Paolo Bonzini wrote:
> I'm thinking of moving more code out of main.c,
> so that the #ifdefs will be only in one or two files.

Here's the sort of thing I had in mind.  It works fine on non-Windows
platforms but I don't know about Windows.  The idea is to move the
Microsoft-specific stuff to a new directory lib/ms.  I'd rather
keep it in a separate directory so that the mainline code is easier
to grep.

A couple of other things.  I assume we can remove m4/djgpp.m4 now?
It seems to be unused.  Also, README.DOS is now wrong, no?
Shouldn't it be removed too, or rewritten from scratch, or
something?

>From 1bc20aada5bbd57e5edea7bb766d17437726ec82 Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Wed, 28 Dec 2011 11:31:21 -0800
Subject: [PATCH] ms: move Microsoft-specific stuf to lib/ms

* src/main.c (hstdout, norm_attr, w32_console_init, w32_sgr2attr)
(pr_sgr_start, w32_clreol, rr_sgr_end) [__MINGW32__]:
Move to lib/ms/colorize.h.
(colorize_init): Renamed from w32_console_init; caller changed.
(grepdir): Use (new) SAME_INODE instead of a heuristic.
(should_colorize): Move to colorize.h.
* lib/ms/colorize.h, src/colorize.h: New files, taken from src/main.c.
The usual one is src/colorize.h; lib/ms/colorize.h overrides it
on Microsoft platforms.
* lib/ms/same-inode.h: New file, to override lib/same-inode.h on
Microsoft platforms.
---
 lib/ms/colorize.h   |  211 +++++++++++++++++++++++++++++++++++++++++++++
 lib/ms/same-inode.h |    2 +
 src/colorize.h      |   50 +++++++++++
 src/main.c          |  239 ++-------------------------------------------------
 4 files changed, 271 insertions(+), 231 deletions(-)
 create mode 100644 lib/ms/colorize.h
 create mode 100644 lib/ms/same-inode.h
 create mode 100644 src/colorize.h

diff --git a/lib/ms/colorize.h b/lib/ms/colorize.h
new file mode 100644
index 0000000..44cc476
--- /dev/null
+++ b/lib/ms/colorize.h
@@ -0,0 +1,211 @@
+/* Support for colorization on MS-Windows console.
+   Copyright 2011 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 3, 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, Inc., 51 Franklin Street - Fifth Floor, Boston, MA
+   02110-1301, USA.  */
+
+/* Written by Eli Zaretskii.  */
+
+#undef DATADIR /* conflicts with objidl.h, which is included by windows.h */
+#include <windows.h>
+
+static HANDLE hstdout = INVALID_HANDLE_VALUE;
+static SHORT norm_attr;
+
+/* Initialize the normal text attribute used by the console.  */
+static void
+colorize_init (void)
+{
+  CONSOLE_SCREEN_BUFFER_INFO csbi;
+
+  hstdout = GetStdHandle (STD_OUTPUT_HANDLE);
+  if (hstdout != INVALID_HANDLE_VALUE
+      && GetConsoleScreenBufferInfo (hstdout, &csbi))
+     norm_attr = csbi.wAttributes;
+  else
+    hstdout = INVALID_HANDLE_VALUE;
+}
+
+/* Return non-zero if we should highlight matches in output.  */
+static int
+should_colorize (int fd)
+{
+  if (! isatty (fd))
+    return 0;
+  /* Windows isatty returns non-zero for the null device too.  */
+  else if (lseek (fd, SEEK_CUR, 0) != -1)
+    return 0;
+  else
+    {
+      /* $TERM is not normally defined on DOS/Windows, so don't require
+         it for highlighting.  But some programs, like Emacs, do define
+         it when running Grep as a subprocess, so make sure they don't
+         set TERM=dumb.  */
+      char const *t = getenv ("TERM");
+      return ! (t && STREQ (t, "dumb"));
+    }
+}
+
+/* Convert a color spec, a semi-colon separated list of the form
+   "NN;MM;KK;...", where each number is a value of the SGR parameter,
+   into the corresponding Windows console text attribute.
+
+   This function supports a subset of the SGR rendition aspects that
+   the Windows console can display.  */
+static int
+w32_sgr2attr (const char *sgr_seq)
+{
+  const char *s, *p;
+  int code, fg = norm_attr & 15, bg = norm_attr & (15 << 4);
+  int bright = 0, inverse = 0;
+  static const int fg_color[] = {
+    0,                 /* black */
+    FOREGROUND_RED,    /* red */
+    FOREGROUND_GREEN,  /* green */
+    FOREGROUND_GREEN | FOREGROUND_RED, /* yellow */
+    FOREGROUND_BLUE,                  /* blue */
+    FOREGROUND_BLUE | FOREGROUND_RED,  /* magenta */
+    FOREGROUND_BLUE | FOREGROUND_GREEN, /* cyan */
+    FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE /* gray */
+  };
+  static const int bg_color[] = {
+    0,                 /* black */
+    BACKGROUND_RED,    /* red */
+    BACKGROUND_GREEN,  /* green */
+    BACKGROUND_GREEN | BACKGROUND_RED, /* yellow */
+    BACKGROUND_BLUE,                  /* blue */
+    BACKGROUND_BLUE | BACKGROUND_RED,  /* magenta */
+    BACKGROUND_BLUE | BACKGROUND_GREEN, /* cyan */
+    BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE /* gray */
+  };
+
+  for (s = p = sgr_seq; *s; p++)
+    {
+      if (*p == ';' || *p == '\0')
+        {
+          code = strtol (s, NULL, 10);
+          s = p + (*p != '\0');
+
+          switch (code)
+            {
+            case 0:    /* all attributes off */
+              fg = norm_attr & 15;
+              bg = norm_attr & (15 << 4);
+              bright = 0;
+              inverse = 0;
+              break;
+            case 1:    /* intensity on */
+              bright = 1;
+              break;
+            case 7:    /* inverse video */
+              inverse = 1;
+              break;
+            case 22:   /* intensity off */
+              bright = 0;
+              break;
+            case 27:   /* inverse off */
+              inverse = 0;
+              break;
+            case 30: case 31: case 32: case 33: /* foreground color */
+            case 34: case 35: case 36: case 37:
+              fg = fg_color[code - 30];
+              break;
+            case 39:   /* default foreground */
+              fg = norm_attr & 15;
+              break;
+            case 40: case 41: case 42: case 43: /* background color */
+            case 44: case 45: case 46: case 47:
+              bg = bg_color[code - 40];
+              break;
+            case 49:   /* default background */
+              bg = norm_attr & (15 << 4);
+              break;
+            default:
+              break;
+            }
+        }
+    }
+  if (inverse)
+    {
+      int t = fg;
+      fg = (bg >> 4);
+      bg = (t << 4);
+    }
+  if (bright)
+    fg |= FOREGROUND_INTENSITY;
+
+  return (bg & (15 << 4)) | (fg & 15);
+}
+
+/* Start displaying text according to the spec in SGR_SEQ, but only if
+   SGR_SEQ is non-empty and COND is non-zero.  If stdout is connected
+   to a console, set the console text attribute; otherwise, emit the
+   SGR escape sequence as on Posix platforms (this is needed when Grep
+   is invoked as a subprocess of another program, such as Emacs, which
+   will handle the display of the matches).  */
+static void
+pr_sgr_start (const char *sgr_seq, int cond)
+{
+  if (cond && *sgr_seq)
+    {
+      if (hstdout != INVALID_HANDLE_VALUE)
+        {
+          SHORT attr = w32_sgr2attr (sgr_seq);
+          SetConsoleTextAttribute (hstdout, attr);
+        }
+      else
+        printf (sgr_start, sgr_seq);
+    }
+}
+
+/* Clear to the end of the current line with the default attribute.
+   This is needed for reasons similar to those that require the "EL to
+   Right after SGR" operation on Posix platforms: if we don't do this,
+   setting the `mt', `ms', or `mc' capabilities to use a non-default
+   background color spills that color to the empty space at the end of
+   the last screen line in a match whose line spans multiple screen
+   lines.  */
+static void
+w32_clreol (void)
+{
+  DWORD nchars;
+  COORD start_pos;
+  DWORD written;
+  CONSOLE_SCREEN_BUFFER_INFO csbi;
+
+  GetConsoleScreenBufferInfo (hstdout, &csbi);
+  start_pos = csbi.dwCursorPosition;
+  nchars = csbi.dwSize.X - start_pos.X;
+
+  FillConsoleOutputAttribute (hstdout, norm_attr, nchars, start_pos,
+                              &written);
+  FillConsoleOutputCharacter (hstdout, ' ', nchars, start_pos, &written);
+}
+
+/* Restore the normal text attribute.  */
+static void
+pr_sgr_end (const char *sgr_seq, int cond)
+{
+  if (cond && *sgr_seq)
+    {
+      if (hstdout != INVALID_HANDLE_VALUE)
+        {
+          SetConsoleTextAttribute (hstdout, norm_attr);
+          w32_clreol ();
+        }
+      else
+        printf ("%s", sgr_end);
+    }
+}
diff --git a/lib/ms/same-inode.h b/lib/ms/same-inode.h
new file mode 100644
index 0000000..ccf5528
--- /dev/null
+++ b/lib/ms/same-inode.h
@@ -0,0 +1,2 @@
+/* Whether two struct stat values are from the same file is unknown.  */
+#define SAME_INODE(a, b) (-1)
diff --git a/src/colorize.h b/src/colorize.h
new file mode 100644
index 0000000..ec9c77e
--- /dev/null
+++ b/src/colorize.h
@@ -0,0 +1,50 @@
+/* Support for colorization on MS-Windows console.
+   Copyright 2011 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 3, 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, Inc., 51 Franklin Street - Fifth Floor, Boston, MA
+   02110-1301, USA.  */
+
+static void
+colorize_init (void)
+{
+}
+
+/* Return non-zero if we should highlight matches in output.  */
+static int
+should_colorize (int fd)
+{
+  if (! isatty (fd))
+    return 0;
+  else
+    {
+      char const *t = getenv ("TERM");
+      return t && !STREQ (t, "dumb");
+    }
+}
+
+static void
+pr_sgr_start (const char *sgr_seq, int cond)
+{
+  if (cond && *sgr_seq)
+    printf (sgr_start, sgr_seq);
+}
+
+
+/* Restore the normal text attribute.  */
+static void
+pr_sgr_end (const char *sgr_seq, int cond)
+{
+  if (cond && *sgr_seq)
+    printf ("%s", sgr_end);
+}
diff --git a/src/main.c b/src/main.c
index ca6f85f..53b3725 100644
--- a/src/main.c
+++ b/src/main.c
@@ -268,198 +268,7 @@ static const struct color_cap color_dict[] =
     { NULL, NULL,                  NULL }
   };
 
-#ifdef __MINGW32__
-/* Support for colorization on MS-Windows console.  */
-
-#undef DATADIR /* conflicts with objidl.h, which is included by windows.h */
-#include <windows.h>
-
-static HANDLE hstdout = INVALID_HANDLE_VALUE;
-static SHORT norm_attr;
-
-/* Initialize the normal text attribute used by the console.  */
-static void
-w32_console_init (void)
-{
-  CONSOLE_SCREEN_BUFFER_INFO csbi;
-
-  hstdout = GetStdHandle (STD_OUTPUT_HANDLE);
-  if (hstdout != INVALID_HANDLE_VALUE
-      && GetConsoleScreenBufferInfo (hstdout, &csbi))
-     norm_attr = csbi.wAttributes;
-  else
-    hstdout = INVALID_HANDLE_VALUE;
-}
-
-/* Convert a color spec, a semi-colon separated list of the form
-   "NN;MM;KK;...", where each number is a value of the SGR parameter,
-   into the corresponding Windows console text attribute.
-
-   This function supports a subset of the SGR rendition aspects that
-   the Windows console can display.  */
-static int
-w32_sgr2attr (const char *sgr_seq)
-{
-  const char *s, *p;
-  int code, fg = norm_attr & 15, bg = norm_attr & (15 << 4);
-  int bright = 0, inverse = 0;
-  static const int fg_color[] = {
-    0,                 /* black */
-    FOREGROUND_RED,    /* red */
-    FOREGROUND_GREEN,  /* green */
-    FOREGROUND_GREEN | FOREGROUND_RED, /* yellow */
-    FOREGROUND_BLUE,                  /* blue */
-    FOREGROUND_BLUE | FOREGROUND_RED,  /* magenta */
-    FOREGROUND_BLUE | FOREGROUND_GREEN, /* cyan */
-    FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE /* gray */
-  };
-  static const int bg_color[] = {
-    0,                 /* black */
-    BACKGROUND_RED,    /* red */
-    BACKGROUND_GREEN,  /* green */
-    BACKGROUND_GREEN | BACKGROUND_RED, /* yellow */
-    BACKGROUND_BLUE,                  /* blue */
-    BACKGROUND_BLUE | BACKGROUND_RED,  /* magenta */
-    BACKGROUND_BLUE | BACKGROUND_GREEN, /* cyan */
-    BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE /* gray */
-  };
-
-  for (s = p = sgr_seq; *s; p++)
-    {
-      if (*p == ';' || *p == '\0')
-        {
-          code = strtol (s, NULL, 10);
-          s = p + (*p != '\0');
-
-          switch (code)
-            {
-            case 0:    /* all attributes off */
-              fg = norm_attr & 15;
-              bg = norm_attr & (15 << 4);
-              bright = 0;
-              inverse = 0;
-              break;
-            case 1:    /* intensity on */
-              bright = 1;
-              break;
-            case 7:    /* inverse video */
-              inverse = 1;
-              break;
-            case 22:   /* intensity off */
-              bright = 0;
-              break;
-            case 27:   /* inverse off */
-              inverse = 0;
-              break;
-            case 30: case 31: case 32: case 33: /* foreground color */
-            case 34: case 35: case 36: case 37:
-              fg = fg_color[code - 30];
-              break;
-            case 39:   /* default foreground */
-              fg = norm_attr & 15;
-              break;
-            case 40: case 41: case 42: case 43: /* background color */
-            case 44: case 45: case 46: case 47:
-              bg = bg_color[code - 40];
-              break;
-            case 49:   /* default background */
-              bg = norm_attr & (15 << 4);
-              break;
-            default:
-              break;
-            }
-        }
-    }
-  if (inverse)
-    {
-      int t = fg;
-      fg = (bg >> 4);
-      bg = (t << 4);
-    }
-  if (bright)
-    fg |= FOREGROUND_INTENSITY;
-
-  return (bg & (15 << 4)) | (fg & 15);
-}
-
-/* Start displaying text according to the spec in SGR_SEQ, but only if
-   SGR_SEQ is non-empty and COND is non-zero.  If stdout is connected
-   to a console, set the console text attribute; otherwise, emit the
-   SGR escape sequence as on Posix platforms (this is needed when Grep
-   is invoked as a subprocess of another program, such as Emacs, which
-   will handle the display of the matches).  */
-static void
-pr_sgr_start (const char *sgr_seq, int cond)
-{
-  if (cond && *sgr_seq)
-    {
-      if (hstdout != INVALID_HANDLE_VALUE)
-        {
-          SHORT attr = w32_sgr2attr (sgr_seq);
-          SetConsoleTextAttribute (hstdout, attr);
-        }
-      else
-        printf (sgr_start, sgr_seq);
-    }
-}
-
-/* Clear to the end of the current line with the default attribute.
-   This is needed for reasons similar to those that require the "EL to
-   Right after SGR" operation on Posix platforms: if we don't do this,
-   setting the `mt', `ms', or `mc' capabilities to use a non-default
-   background color spills that color to the empty space at the end of
-   the last screen line in a match whose line spans multiple screen
-   lines.  */
-static void
-w32_clreol (void)
-{
-  DWORD nchars;
-  COORD start_pos;
-  DWORD written;
-  CONSOLE_SCREEN_BUFFER_INFO csbi;
-
-  GetConsoleScreenBufferInfo (hstdout, &csbi);
-  start_pos = csbi.dwCursorPosition;
-  nchars = csbi.dwSize.X - start_pos.X;
-
-  FillConsoleOutputAttribute (hstdout, norm_attr, nchars, start_pos,
-                              &written);
-  FillConsoleOutputCharacter (hstdout, ' ', nchars, start_pos, &written);
-}
-
-/* Restore the normal text attribute.  */
-static void
-pr_sgr_end (const char *sgr_seq, int cond)
-{
-  if (cond && *sgr_seq)
-    {
-      if (hstdout != INVALID_HANDLE_VALUE)
-        {
-          SetConsoleTextAttribute (hstdout, norm_attr);
-          w32_clreol ();
-        }
-      else
-        printf ("%s", sgr_end);
-    }
-}
-#else
-
-static void
-pr_sgr_start (const char *sgr_seq, int cond)
-{
-  if (cond && *sgr_seq)
-    printf (sgr_start, sgr_seq);
-}
-
-
-/* Restore the normal text attribute.  */
-static void
-pr_sgr_end (const char *sgr_seq, int cond)
-{
-  if (cond && *sgr_seq)
-    printf ("%s", sgr_end);
-}
-#endif /* __MINGW32__ */
+#include <colorize.h>
 
 static struct exclude *excluded_patterns;
 static struct exclude *included_patterns;
@@ -1509,20 +1318,14 @@ grepdir (char const *dir, struct stats const *stats)
       && excluded_file_name (excluded_directory_patterns, dir))
     return 1;
 
-  /* Mingw32 does not support st_ino.  No known working hosts use zero
-     for st_ino, so assume that the Mingw32 bug applies if it's zero.  */
-  if (stats->stat.st_ino)
+  for (ancestor = stats; (ancestor = ancestor->parent) != 0;  )
     {
-      for (ancestor = stats; (ancestor = ancestor->parent) != 0;  )
+      if (0 < SAME_INODE (ancestor->stat, stats->stat))
         {
-          if (ancestor->stat.st_ino == stats->stat.st_ino
-              && ancestor->stat.st_dev == stats->stat.st_dev)
-            {
-              if (!suppress_errors)
-                error (0, 0, _("warning: %s: %s"), dir,
-                       _("recursive directory loop"));
-              return 1;
-            }
+          if (!suppress_errors)
+            error (0, 0, _("warning: %s: %s"), dir,
+                   _("recursive directory loop"));
+          return 1;
         }
     }
 
@@ -1941,30 +1744,6 @@ parse_grep_colors (void)
                  "at remaining substring \"%s\""), p, q);
 }
 
-/* Return non-zero if we should highlight matches in output. */
-static int
-should_colorize (int fd)
-{
-  const char *t;
-
-#if defined __MINGW32__ || defined __DJGPP__
-  return
-    isatty (fd)
-#ifdef __MINGW32__
-    /* Without the lseek call, Windows isatty returns non-zero for the
-       null device as well.  */
-    && lseek (fd, SEEK_CUR, 0) == -1
-#endif
-    /* $TERM is not normally defined on DOS/Windows, so don't require
-       it for highlighting.  But some programs, like Emacs, do define
-       it when running Grep as a subprocess, so make sure they don't
-       set TERM=dumb.  */
-    && !((t = getenv ("TERM")) && STREQ (t, "dumb"));
-#else  /* not __MINGW32__, not __DJGPP__ */
-  return isatty (fd) && (t = getenv ("TERM")) && !STREQ (t, "dumb");
-#endif
-}
-
 int
 main (int argc, char **argv)
 {
@@ -2297,9 +2076,7 @@ main (int argc, char **argv)
 
   if (color_option == 2)
     color_option = should_colorize (STDOUT_FILENO);
-#ifdef __MINGW32__
-  w32_console_init ();
-#endif
+  colorize_init ();
 
   /* POSIX.2 says that -q overrides -l, which in turn overrides the
      other output options.  */
-- 
1.7.6.4