Re: GNU patch: upcoming stable release; call for testing

[Eric Blake]

On 05/03/2010 04:29 PM, Andreas Gruenbacher wrote:
> I am pleased to announce that there is progress towards the next stable 
> release of GNU patch.  This is a call for testing so that things will work as 
> expected, on as many platforms as possible.

You've got some bugs detected by compiler warnings on cygwin:

In file included from patch.c:32:
./util.h: In function skip_spaces:
./util.h:79: error: array subscript has type char
patch.c: In function make_temp:
patch.c:1590: error: call to mktemp declared with attribute warning: the
use of `mktemp' is dangerous; use `mkstemp' instead
pch.c: In function open_patch_file:
pch.c:115: warning: implicit declaration of function setmode
pch.c: In function intuit_diff_type:
pch.c:566: warning: array subscript has type char
util.c: In function parse_name:
util.c:1452: warning: array subscript has type char
util.c:1460: warning: array subscript has type char


Here's a potential patch for util.h, pch.c, and util.c (without it, some
locales with single-byte characters where some 8-bit characters are
spaces will be mishandled).

You should really consider using the gnulib mkstemp module to guarantee
that patch.c's make_temp() can blindly use the secure mkstemp() instead
of wavering between mktemp() and tmpfile(), where the former is insecure
and the latter has some puny limits on some systems and is permitted by
POSIX to write spurious output to stderr on failure.  Or go one step
further and use gnulib's clean-temp module, which guarantees automatic
cleanup of temporary files even in the face of signals (unlike raw use
of mkstemp()).

The testsuite also makes an unportable assumption; tests/read-only-files
fails on cygwin when run as an administrator (basically, with root-like
privileges where all files are readable regardless of mode).  You need
to make that test skip if you detect superuser privileges.

Also, as a nit, it's traditional to put the current copyright owner
first, not last.  Emacs' copyright-mode only offers to list the current
year in the first copyright line found; when I was preparing this patch,
that meant that emacs kept trying to add the year 2010 to Larry Wall
instead of FSF.

From 66b5c78d60b6a52f144cbb5b967530491e867ae7 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 4 May 2010 09:33:12 -0600
Subject: [PATCH] build: silence some cygwin warnings

* src/pch.c (includes): Add <io.h> for setmode.
(intuit_diff_type): Avoid 8-bit problems.
* src/util.c (parse_name): Likewise.
* src/util.h (skip_spaces): Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog  |   12 ++++++++++--
 src/pch.c  |    5 ++++-
 src/util.c |    4 ++--
 src/util.h |    2 +-
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d25482b..83cef1a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2010-05-04  Eric Blake  <address@hidden>
+
+       build: silence some cygwin warnings
+       * src/pch.c (includes): Add <io.h> for setmode.
+       (intuit_diff_type): Avoid 8-bit problems.
+       * src/util.c (parse_name): Likewise.
+       * src/util.h (skip_spaces): Likewise.
+
 2009-05-04  Andreas Gruenbacher  <address@hidden>

        * tests/test-lib.sh: Flag tests with missing pre-requirements as
@@ -3440,13 +3448,13 @@ Sun Dec 17 17:29:48 1989  Jim Kingdon  (kingdon
at hobbes.ai.mit.edu)
 Copyright (C) 1984, 1985, 1986, 1987, 1988 Larry Wall.

 Copyright (C) 1989, 1990, 1991, 1992, 1993, 1997, 1998, 1999, 2000, 2001,
-2002, 2009 Free Software Foundation, Inc.
+2002, 2009, 2010 Free Software Foundation, Inc.

 This file is part of GNU Patch.

 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 2, or (at your option)
+the Free Software Foundation; either version 3, or (at your option)
 any later version.

 This program is distributed in the hope that they will be useful,
diff --git a/src/pch.c b/src/pch.c
index 7e35e68..6a4941e 100644
--- a/src/pch.c
+++ b/src/pch.c
@@ -27,6 +27,9 @@
 #undef XTERN
 #define XTERN
 #include <pch.h>
+#if HAVE_SETMODE_DOS
+# include <io.h>
+#endif

 #define INITHUNKMAX 125                        /* initial dynamic allocation 
size */

@@ -563,7 +566,7 @@ intuit_diff_type (bool need_header, mode_t *p_file_type)
                p_name[i] = 0;
              }
            if (! ((p_name[OLD] = parse_name (s + 11, strippath, &u))
-                  && ISSPACE (*u)
+                  && ISSPACE ((unsigned char) *u)
                   && (p_name[NEW] = parse_name (u, strippath, &u))
                   && (u = skip_spaces (u), ! *u)))
              for (i = OLD; i <= NEW; i++)
diff --git a/src/util.c b/src/util.c
index 33f8d63..c6e8263 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1449,7 +1449,7 @@ parse_name (char const *s, int strip_leading, char
const **endp)
 {
   char *ret;

-  while (ISSPACE (*s))
+  while (ISSPACE ((unsigned char) *s))
     s++;
   if (*s == '"')
     ret = parse_c_string (s, endp);
@@ -1457,7 +1457,7 @@ parse_name (char const *s, int strip_leading, char
const **endp)
     {
       char const *t;

-      for (t = s; *t && ! ISSPACE (*t); t++)
+      for (t = s; *t && ! ISSPACE ((unsigned char) *t); t++)
        /* do nothing*/ ;
       ret = savebuf (s, t - s + 1);
       ret[t - s] = 0;
diff --git a/src/util.h b/src/util.h
index d8b851d..2ba8f36 100644
--- a/src/util.h
+++ b/src/util.h
@@ -76,7 +76,7 @@ void set_file_attributes (char const *, enum
file_attributes, struct stat *,
 static inline char const *
 skip_spaces (char const *str)
 {
-  while (ISSPACE (*str))
+  while (ISSPACE ((unsigned char) *str))
     str++;
   return str;
 }
-- 
1.7.0.4

signature.asc
Description: OpenPGP digital signature