[bug-patch] Re: GNU patch: upcoming stable release; call for testing

[Andreas Gruenbacher]

Eric,

thanks for the careful review and patch.

On Tuesday 04 May 2010 17:35:35 Eric Blake wrote:
> 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).

Oh, good catch.  This whole ISSPACE() stuff should be cleaned up too one day 
... just not now :)

> 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()).

Should be fixed too, I agree.

> 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.

Let's see if  test `id -u` = 0  will help.

> 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.

Okay, fine.

Thanks,
Andreas