Re: Recognize Mercurial in build-aux/bootstrap
On 14-Nov-2009, Jim Meyering wrote: | John W. Eaton wrote: | Would the following patch be OK for recognizing Mercurial in the | build-aux/bootstrap script? It seems to work for me. Maybe there is | a better way of recognizing that a file is tracked, but I'm not sure | what it is. | | Can you investigate that? | It'd be good to have a definitive answer. I'll try to find out. | Unfortunately, hg status FILE exits with a status of 0 | even when FILE does not exist. | | I'll look more carefully later. | First, two details: please send patches in git format-patch format | (if you're new to git/gnulib guidelines, see these | http://git.sv.gnu.org/cgit/coreutils.git/plain/HACKING | though note that in gnulib we do maintain a ChangeLog file) Sorry, I'm new to git and I wasn't sure how to make a proper changeset. I'll try to follow the guidelines in the future. | Also mentioned in HACKING is the issue of copyright assignment. | Between these two tiny patches, you've reached the limit for someone | with no assignment on file for gnulib. I've requested an assignment form for past and future changes. jwe
better word-list-to-minimized-regexp code?
You may have seen the tests in maint.mk that help you avoid inclusion
of unused header files, e.g.,
# Prohibit the inclusion of assert.h without an actual use of assert.
sc_prohibit_assert_without_use:
@h='assert.h' re='\assert *\(' $(_header_without_use)
That causes make syntax-check to fail if you include assert.h
in a file with no use of assert. Of course, it's a naive check,
and can be tricked by #if-0'd or commented-out code, but then again,
leaving in an unnecessary #include is not serious.
I wanted one for xalloc.h, too, and here's the story:
Extracting the symbols is easy (see below), but converting such a long
word list to a regexp is tedious. So automate that, too. But with what?
I've tried the perl modules, Regexp::Assemble and Regexp::List,
and neither did what I wanted.
Here's the list of the x symbols:
x2realloc
xalloc_die
xalloc_oversized
xcalloc
xmalloc
xmemdup
xrealloc
xstrdup
xzalloc
The modules I tried produced this (or equiv):
x(alloc_(oversized|die)|([cz]|2?re)alloc|m(alloc|emdup)|strdup)
Both botched the inclusion of xmalloc.
They missed the fact that sharing both the short prefix of x and the
suffix of alloc would be better than merely sharing the xm prefix.
Doing it by hand, I get this:
x(alloc_(oversized|die)|([cmz]|2?re)alloc|(mem|str)dup)
If you change the list of inputs via s/xmalloc/xgalloc/,
you see that they get it right:
x(alloc_(oversized|die)|([cgz]|2?re)alloc|(mem|str)dup)
It appears that they are tricked by a local minimum:
xm is a longer shared prefix than merely x
Here's what I did to test Regexp::List:
$ {perl -lne '/^# *define (\w+)\(/ and print $1' lib/xalloc.h|grep -v '^__';
perl -lne '/^(?:extern )?(?:void|char) \*?(\w+) \(/ and print $1'
lib/xalloc.h;}\
|grep '^x' \
|perl -MRegexp::List -le \
'print Regexp::List-new-list2re()'| sed 's/\?://g'
(?-xism:x(alloc_(die|oversized)\
|m(alloc|emdup)\
|((2?re|[cz])alloc|strdup)\
))
For Regexp::Assemble, see the comments below:
From 9a93371caac4a9440c41d3269b21db585920e4c4 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Sat, 14 Nov 2009 09:53:26 +0100
Subject: [PATCH] maint.mk: Prohibit inclusion of xalloc.h without use.
* top/maint.mk (sc_prohibit_close_stream_without_use): New rule.
---
ChangeLog|5 +
top/maint.mk | 17 +
2 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e23f285..c821d8d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2009-11-14 Jim Meyering meyer...@redhat.com
+
+ maint.mk: Prohibit inclusion of xalloc.h without use.
+ * top/maint.mk (sc_prohibit_close_stream_without_use): New rule.
+
2009-11-14 John W. Eaton j...@gnu.org
strftime.h: wrap funtion declaration in extern C block
diff --git a/top/maint.mk b/top/maint.mk
index 73ea8ea..34d66e1 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -289,6 +289,23 @@ sc_prohibit_error_without_use:
re='\error(_at_line|_print_progname|_one_per_line|_message_count)?
*\('\
$(_header_without_use)
+# Don't include xalloc.h unless you use one of its functions.
+# Consider these symbols:
+# perl -lne '/^# *define (\w+)\(/ and print $1' lib/xalloc.h|grep -v '^__';
+# perl -lne '/^(?:extern )?(?:void|char) \*?(\w+) \(/ and print $1'
lib/xalloc.h
+# Divide into two sets on case, and filter each through this:
+# | sort | perl -MRegexp::Assemble -le \
+# 'print Regexp::Assemble-new(file = /dev/stdin)-as_string'|sed
's/\?://g'
+# Note this was produced by the above:
+# _xa1 = x(alloc_(oversized|die)|([cz]|2?re)alloc|m(alloc|emdup)|strdup)
+# But we can do better:
+_xa1 = x(alloc_(oversized|die)|([cmz]|2?re)alloc|(mem|str)dup)
+_xa2 = X([CZ]|N?M)ALLOC
+sc_prohibit_xalloc_without_use:
+ @h='xalloc.h' \
+ re='\($(_xa1)|$(_xa2)) *\('\
+ $(_header_without_use)
+
sc_prohibit_safe_read_without_use:
@h='safe-read.h' re='(\SAFE_READ_ERROR\|\safe_read *\()' \
$(_header_without_use)
--
1.6.5.2.372.gc0502
Re: code coverage report for gnulib
Hi Simon, * Simon Josefsson wrote on Fri, Nov 13, 2009 at 10:41:20AM CET: I prepared a code coverage report for gnulib: http://www.gnu.org/software/gnulib/coverage/ Thank you for doing this! How did you create the report, and was manual hacking needed in order to get it to work? I'm running into some issues with lcov on packages using libtool and having sources in both the source and the build tree, requiring quite some manual symlinking so that they are all found. Thanks, Ralf
tests: temporary files may be left behind
I noticed that when the test-c-stack2.sh test is skipped,
it leaves behind its temporary file.
I propose to fix it by adding a trap ... 0.
A welcome side-effect is that with this change, you remove
temporary files from only one place: the trap, rather
than just prior to every other exit point.
The only tricky part about the trap-0 is that you
have to be sure to preserve the exit status.
Here's the patch for that particular file:
diff --git a/tests/test-c-stack2.sh b/tests/test-c-stack2.sh
index a80373d..88c3136 100755
--- a/tests/test-c-stack2.sh
+++ b/tests/test-c-stack2.sh
@@ -1,7 +1,8 @@
#!/bin/sh
tmpfiles=
-trap 'rm -fr $tmpfiles' 1 2 3 15
+trap '__st=$?; rm -fr $tmpfiles; exit $__st' 0
+trap '__st=$?; (exit $__st); exit $__st' 1 2 3 15
tmpfiles=t-c-stack2.tmp
@@ -31,6 +32,4 @@ else
(exit 1); exit 1
fi
-rm -fr $tmpfiles
-
exit 0
However, that is an idiom repeated in many other files:
$ git grep -l 'trap .rm -fr .tmpfiles. 1 2 3 15'
tests/test-atexit.sh
tests/test-binary-io.sh
tests/test-c-stack.sh
tests/test-closein.sh
tests/test-dprintf-posix.sh
tests/test-fprintf-posix.sh
tests/test-lseek.sh
tests/test-perror.sh
tests/test-printf-posix.sh
tests/test-select-in.sh
tests/test-select-out.sh
tests/test-sigpipe.sh
tests/test-tsearch.sh
tests/test-vdprintf-posix.sh
tests/test-vfprintf-posix.sh
tests/test-vprintf-posix.sh
tests/test-xprintf-posix.sh
tests/test-xstrtoimax.sh
tests/test-xstrtol.sh
tests/test-xstrtoumax.sh
tests/test-yesno.sh
tests/uniwidth/test-uc_width2.sh
So below I've included a patch to correct all of those, too,
using this script:
cat EOF test-trap-convert
eval '(exit $?0)' eval 'exec perl -wS -ni $0 ${1+$@}'
eval 'exec perl -wS -ni $0 $argv:q'
if 0;
use strict;
use warnings;
my $rpl = 'EOF';
trap '__st=$?; rm -fr $tmpfiles; exit $__st' 0
trap '__st=$?; (exit $__st); exit $__st' 1 2 3 15
EOF
/^\# Cleanup$/ and next;
/(.*) \{ rm -[fr]+ \$tmpfiles; (exit 1); \}$/ and (print $1 $2\n), next;
/(.*) rm -[fr]+ \$tmpfiles; (exit 1)$/ and (print $1 $2\n), next;
/(.*) rm -[fr]+ \$tmpfiles; (exit 1;.*)/ and (print $1 $2;), next;
/^trap.*15$/ and (print $rpl), next;
/^\s*rm -[fr]+ \$tmpfiles$/ and next;
print;
EOF
chmod a+x test-trap-convert
I ran it like this:
grep -l 'trap .rm -fr .tmpfiles. 1 2 3 15'|xargs ./test-trap-convert
It's not perfect, because it leaves a few pairs of adjacent blank lines,
but that's easy to fix. I did it with this:
$ perl -pi -0777 -e 's/\n\n\n+/\n\n/g' $(git ls-files -m)
Oops. There's one more nit: there were two now-dangling # Cleanup comments.
So I went back, tweaked the script to remove those, too, and reran things.
Here's the result. Any objections?
Note that the initial regexp by which I selected
the files to update is very strict.
I'd be surprised if it can't be relaxed to catch more,
but I'd like to limit this first pass to those changes
I've already reviewed (visually, and partially tested) a few times.
From ccfa387893628de405055d2ec69ee2f07094424d Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Sat, 14 Nov 2009 12:24:21 +0100
Subject: [PATCH] tests: use a better trap idiom
---
tests/test-atexit.sh |5 ++---
tests/test-binary-io.sh |5 ++---
tests/test-c-stack.sh|5 ++---
tests/test-c-stack2.sh |5 ++---
tests/test-closein.sh|6 ++
tests/test-dprintf-posix.sh |5 ++---
tests/test-fprintf-posix.sh |5 ++---
tests/test-lseek.sh |4 ++--
tests/test-perror.sh | 10 +-
tests/test-printf-posix.sh |5 ++---
tests/test-select-in.sh |5 ++---
tests/test-select-out.sh |5 ++---
tests/test-sigpipe.sh| 10 +-
tests/test-tsearch.sh|7 +++
tests/test-vdprintf-posix.sh |5 ++---
tests/test-vfprintf-posix.sh |5 ++---
tests/test-vprintf-posix.sh |5 ++---
tests/test-xprintf-posix.sh |9 -
tests/test-xstrtoimax.sh |5 ++---
tests/test-xstrtol.sh|5 ++---
tests/test-xstrtoumax.sh |5 ++---
tests/test-yesno.sh |6 ++
tests/uniwidth/test-uc_width2.sh |5 ++---
23 files changed, 55 insertions(+), 77 deletions(-)
diff --git a/tests/test-atexit.sh b/tests/test-atexit.sh
index 49c7729..541fb24 100755
--- a/tests/test-atexit.sh
+++ b/tests/test-atexit.sh
@@ -1,7 +1,8 @@
#!/bin/sh
tmpfiles=
-trap 'rm -fr $tmpfiles' 1 2 3 15
+trap '__st=$?; rm -fr $tmpfiles; exit $__st' 0
+trap '__st=$?; (exit $__st); exit $__st' 1 2 3 15
tmpfiles=$tmpfiles t-atexit.tmp
# Check that an atexit handler is called when main() returns normally.
@@ -27,6 +28,4 @@ if test -f t-atexit.tmp; then
exit 1
fi
-rm -fr $tmpfiles
-
exit 0
diff --git a/tests/test-binary-io.sh b/tests/test-binary-io.sh
index 33e128c..5a30b41 100755
--- a/tests/test-binary-io.sh
+++
syntax-check: copyright-check
copyright-check doesn't handle multi-line copyright years for texinfo,
the following causes an error that the year list is outdated:
Copyright @copyright{} 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007,
2008, 2009 Free Software Foundation, Inc.
output from syntax-check
it would be nice if the output from syntax-check was compatible with compilation mode so that one can use it to jump to each line easily.
Re: output from syntax-check
Alfred M. Szmidt wrote: it would be nice if the output from syntax-check was compatible with compilation mode so that one can use it to jump to each line easily. I agree that it would be nice. Unfortunately, doing that is often at odds with the goals of keeping the test code simple and efficient. If you find small and clean changes that improve matters without inducing inordinate inefficiency even in projects with thousands of source files, please send a patch. Otherwise, I suggest you just write it off, since addressing more than a few syntax-check violations is usually a one-time occurrence.
Re: tests: temporary files may be left behind
Hi Jim, * Jim Meyering wrote on Sat, Nov 14, 2009 at 01:11:38PM CET: I propose to fix it by adding a trap ... 0. A welcome side-effect is that with this change, you remove temporary files from only one place: the trap, rather than just prior to every other exit point. The only tricky part about the trap-0 is that you have to be sure to preserve the exit status. Not only that, you also have to ensure to get the right exit status into the zero trap, by using something like (exit $N); exit $N throughout, see '(autoconf.info)Limitations of Builtins'. Here's the patch for that particular file: It is lacking a few of the above. Cheers, Ralf
[PATCH] test-freading: remove a temporary file
Hi Bruno, Without this change, this test's temporary file is left behind every time it is run. Any objection? From 314ab5c4729f34bcb2aca85181091a4f22d98e52 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Sat, 14 Nov 2009 13:20:25 +0100 Subject: [PATCH] test-freading: remove a temporary file * tests/test-freading.c: Include unistd.h. (main): Unlink temporary file. --- ChangeLog |4 tests/test-freading.c |3 +++ 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/tests/test-freading.c b/tests/test-freading.c index dfa5ffb..4dfe3f3 100644 --- a/tests/test-freading.c +++ b/tests/test-freading.c @@ -22,6 +22,7 @@ #include stdio.h #include stdlib.h +#include unistd.h /* None of the files accessed by this test are large, so disable the fseek link warning if we are not using the gnulib fseek module. */ @@ -170,9 +171,11 @@ main (void) if (fclose (fp)) goto skip; + unlink (TESTFILE); return 0; skip: fprintf (stderr, Skipping test: file operations failed.\n); + unlink (TESTFILE); return 77; } -- 1.6.5.2.372.gc0502
Re: tests: temporary files may be left behind
Ralf Wildenhues wrote:
* Jim Meyering wrote on Sat, Nov 14, 2009 at 01:11:38PM CET:
I propose to fix it by adding a trap ... 0.
A welcome side-effect is that with this change, you remove
temporary files from only one place: the trap, rather
than just prior to every other exit point.
The only tricky part about the trap-0 is that you
have to be sure to preserve the exit status.
Not only that, you also have to ensure to get the right exit status into
the zero trap, by using something like
(exit $N); exit $N
throughout, see '(autoconf.info)Limitations of Builtins'.
Here's the patch for that particular file:
It is lacking a few of the above.
Hi Ralf,
Yes, I noticed those, but my change introduced no new ones.
Such an additional change (if desired) belongs in a separate patch.
It would affect far more scripts that I've targeted.
IMHO, we should start using this Exit function
(I think you're the one who first suggested it),
# We use a trap below for cleanup. This requires us to go through
# hoops to get the right exit status transported through the signal.
# So use `Exit STATUS' instead of `exit STATUS' inside of the tests.
# Turn off errexit here so that we don't trip the bug with OSF1/Tru64
# sh inside this function.
Exit ()
{
set +e
(exit $1)
exit $1
}
but that would require support for functions, which
some of these tests don't yet assume.
Actually, I'd like to make each and every test
source some other file with enough infrastructure
to ensure functions are supported, and things like
temp-dir creation and removal are handled automatically.
We really shouldn't be duplicating all of that in every
single test script.
However, I seem to recall that some people here didn't
want to do that.
Re: tests: temporary files may be left behind
* Jim Meyering wrote on Sat, Nov 14, 2009 at 01:33:14PM CET: Ralf Wildenhues wrote: Not only that, you also have to ensure to get the right exit status into the zero trap, by using something like (exit $N); exit $N throughout, see '(autoconf.info)Limitations of Builtins'. Yes, I noticed those, but my change introduced no new ones. Such an additional change (if desired) belongs in a separate patch. But that separate patch would belong before your proposed one, as the zero trap exposes this issue more. Cheers, Ralf
Re: output from syntax-check
* Jim Meyering wrote on Sat, Nov 14, 2009 at 01:17:19PM CET: Unfortunately, doing that is often at odds with the goals of keeping the test code simple and efficient. If you find small and clean changes that improve matters without inducing inordinate inefficiency even in projects with thousands of source files, please send a patch. Proposed sed extension to help this: http://lists.gnu.org/archive/html/bug-gnu-utils/2009-11/msg00030.html
Re: pending patches?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 11/14/2009 12:20 AM: Eventually, it'd be good to factor out your nap function definition, maybe into a module. There is already one copy in test-stat-time.c, and this patch adds two more. I presume they're all identical. I thought about that. They weren't quite identical, in that they need to know what file name to probe without interfering with other parallel tests. I also plan to use both test-chown.h and test-lchown.h from within the soon-to-appear test-fchownat.c, so yes, some factoring would be nice. I'm also thinking of adding a usleep module to guarantee Linux semantics (usleep(100) waits a second, rather than failing instantly with EINVAL). That will clean up its use somewhat. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr+rh8ACgkQ84KuGfSFAYBNZACguhUPEcpOaj5gsfv7tLVs7edg yH0Anj32tFC8MfEwUkmxXu95MMPkF6By =695y -END PGP SIGNATURE-
Re: [PATCH] bootstrap: sync from coreutils
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 11/13/2009 11:46 PM: With the patch I committed, ./bootstrap --gnulib-srcdir=$HOME/w/co/gnulib now updates .gitmodules (as indicated by your diff below). Before it did not. Or if $GNULIB_SRCDIR is set in your environment. But that's exactly what I do in my ~/.bashrc, since other projects (like tar) that aren't on the latest-and-greatest version of gnulib's bootstrap still go looking at $GNULIB_SRCDIR for guidance. Which means every time I re-bootstrap coreutils, it is picking up on my environment variable, and creating a dirty tree. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr+rpAACgkQ84KuGfSFAYBTIACgx52doKev178Itr7n26IPnEIy eOwAn14gAr0iPD1aCEo0lMBAo/GBW1NL =l/J5 -END PGP SIGNATURE-
Re: output from syntax-check
Unfortunately, doing that is often at odds with the goals of keeping the test code simple and efficient. If you find small and clean changes that improve matters without inducing inordinate inefficiency even in projects with thousands of source files, please send a patch. Proposed sed extension to help this: http://lists.gnu.org/archive/html/bug-gnu-utils/2009-11/msg00030.html This is a good aproach, but for compilation-mode to work you need the line number as well. Would it be acceptable for syntax-check to use this option if it is included in GNU sed? It wouldn't be compatible with POSIX sed, but one could do a check for the option and only use it if one is using GNU sed.
Re: [PATCH] Fix $m4dirs count
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Robert Millan on 11/9/2009 2:00 PM: This is wrong, since 'echo -n' is not portable. It should use printf instead, or find some other portable way to count $m4dirs. Hi, In that case, please use printf. It should work too. I was waiting for Bruno to reply, but your patch looks safe (with printf) and gnulib-tool already uses printf. So I'm pushing: - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr+tOMACgkQ84KuGfSFAYCOswCgnYCtjj2M8HSq8L0/fmycdBTz 9voAnR5LStPnExXRJojQXWi7rK/R7wfM =Q2j1 -END PGP SIGNATURE- From fb1b608caaa3de195f3d97356e95a165ef5517da Mon Sep 17 00:00:00 2001 From: Robert Millan rmh.g...@aybabtu.com Date: Sat, 14 Nov 2009 06:45:02 -0700 Subject: [PATCH] gnulib-tool: correctly detect absence of m4 directories $m4dirs is incorrectly counting. In my particular case (correct value: 0, detected value: 1), this resulted in gnulib-tool silently exitting with no visible error (and no job done). * gnulib-tool: Avoid extra newline on data passed to wc -l. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |5 + gnulib-tool |2 +- 2 files changed, 6 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index c821d8d..5fbff63 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2009-11-14 Robert Millan rmh.g...@aybabtu.com (tiny change) + + gnulib-tool: correctly detect absence of m4 directories + * gnulib-tool: Avoid extra newline on data passed to wc -l. + 2009-11-14 Jim Meyering meyer...@redhat.com maint.mk: Prohibit inclusion of xalloc.h without use. diff --git a/gnulib-tool b/gnulib-tool index 397f442..aafd345 100755 --- a/gnulib-tool +++ b/gnulib-tool @@ -4500,7 +4500,7 @@ case $mode in sedexpr2='s,^[^/]*$,.,' sedexpr3='s,/[^/]*$,,' m4dirs=`sed -n -e $sedexpr1 aclocal.m4 | sed -e $sedexpr2 -e $sedexpr3 | LC_ALL=C sort -u` - m4dirs_count=`echo $m4dirs | wc -l` + m4dirs_count=`printf %s $m4dirs | wc -l` fi fi if test $m4dirs_count = 0; then -- 1.6.5.rc1
Re: tests: temporary files may be left behind
Ralf Wildenhues wrote: * Jim Meyering wrote on Sat, Nov 14, 2009 at 01:33:14PM CET: Ralf Wildenhues wrote: Not only that, you also have to ensure to get the right exit status into the zero trap, by using something like (exit $N); exit $N throughout, see '(autoconf.info)Limitations of Builtins'. Yes, I noticed those, but my change introduced no new ones. Such an additional change (if desired) belongs in a separate patch. But that separate patch would belong before your proposed one, as the zero trap exposes this issue more. If someone actually objects, I'll change each exit N to (exit N); exit N in the affected files, but I think it's not worthwhile, since few people test on OSF1/Tru64, and even fewer use ash for /bin/sh.
Re: tests: temporary files may be left behind
* Jim Meyering wrote on Sat, Nov 14, 2009 at 02:51:34PM CET: Ralf Wildenhues wrote: But that separate patch would belong before your proposed one, as the zero trap exposes this issue more. If someone actually objects, I'll change each exit N to (exit N); exit N in the affected files, but I think it's not worthwhile, since few people test on OSF1/Tru64, and even fewer use ash for /bin/sh. Doesn't this hit you with zsh and with Solaris /bin/sh, too?
xalloc.h drags in dependencies
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
I was surprised when I got a link failure when using the Solaris cc on
test-chown.c due to missing program_name, even though using gcc did just
fine. It turns out that mgetgroups.c uses xalloc_oversized from
xalloc.h, which is only a macro and by itself drags nothing in; but the
rest of xalloc.h includes four inline functions which in turn drag in
references to xalloc_die, error, program_name, and thus the link failure.
I thought about changing the #if HAVE_INLINE part of xalloc.h to instead
be #if HAVE_INLINE defined __GNUC__, but this penalizes other
compilers. I don't want to just open-code xalloc_oversized into
mgetgroups.c. So my idea was to move the one portion of xalloc.h that
does not depend on xalloc_die into a header that is already LGPL; that
way, mgetgroups would depend only on xsize, not xalloc. So, how does this
patch look?
- --
Don't work too hard, make some time for fun as well!
Eric Blake e...@byu.net
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEUEARECAAYFAkr+vyAACgkQ84KuGfSFAYBDUQCXaDqUzoyhSbyITU9+xg//Ny/g
NgCcC1BMCjY02xtKSwAey4JQD3KSI0w=
=64dy
-END PGP SIGNATURE-
From 2540d680580bcdde870fc24aa748f0c749f02d8d Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Sat, 14 Nov 2009 07:24:39 -0700
Subject: [PATCH 1/2] xalloc: move xalloc_oversized to xsize
xalloc_oversized was the only thing in xalloc.h that did not
depend on xalloc_die. But due to inline functions in xalloc.h,
Solaris cc gives a link failure if you use only xalloc_oversized
and don't provide xalloc_die. Moving it to the LGPL header
xsize.h can allow other modules to break the link dependency.
* modules/xalloc (Depends-on): Add xsize.
* lib/xalloc.h (xalloc_oversized): Move...
* lib/xsize.h: ...here.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |7 +++
lib/xalloc.h | 19 +++
lib/xsize.h| 17 -
modules/xalloc |1 +
4 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 5fbff63..5ddacae 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2009-11-14 Eric Blake e...@byu.net
+
+ xalloc: move xalloc_oversized to xsize
+ * modules/xalloc (Depends-on): Add xsize.
+ * lib/xalloc.h (xalloc_oversized): Move...
+ * lib/xsize.h: ...here.
+
2009-11-14 Robert Millan rmh.g...@aybabtu.com (tiny change)
gnulib-tool: correctly detect absence of m4 directories
diff --git a/lib/xalloc.h b/lib/xalloc.h
index 57a13e0..b3166b4 100644
--- a/lib/xalloc.h
+++ b/lib/xalloc.h
@@ -1,7 +1,8 @@
/* xalloc.h -- malloc with out-of-memory checking
Copyright (C) 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998,
- 1999, 2000, 2003, 2004, 2006, 2007, 2008 Free Software Foundation, Inc.
+ 1999, 2000, 2003, 2004, 2006, 2007, 2008, 2009 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
@@ -21,6 +22,7 @@
# include stddef.h
+# include xsize.h /* for xalloc_oversized */
# ifdef __cplusplus
extern C {
@@ -60,21 +62,6 @@ void *x2realloc (void *p, size_t *pn);
void *xmemdup (void const *p, size_t s) ATTRIBUTE_MALLOC;
char *xstrdup (char const *str) ATTRIBUTE_MALLOC;
-/* Return 1 if an array of N objects, each of size S, cannot exist due
- to size arithmetic overflow. S must be positive and N must be
- nonnegative. This is a macro, not an inline function, so that it
- works correctly even when SIZE_MAX N.
-
- By gnulib convention, SIZE_MAX represents overflow in size
- calculations, so the conservative dividend to use here is
- SIZE_MAX - 1, since SIZE_MAX might represent an overflowed value.
- However, malloc (SIZE_MAX) fails on all known hosts where
- sizeof (ptrdiff_t) = sizeof (size_t), so do not bother to test for
- exactly-SIZE_MAX allocations on such hosts; this avoids a test and
- branch when S is known to be 1. */
-# define xalloc_oversized(n, s) \
-((size_t) (sizeof (ptrdiff_t) = sizeof (size_t) ? -1 : -2) / (s) (n))
-
/* In the following macros, T must be an elementary or structure/union or
typedef'ed type, or a pointer to such a type. To apply one of the
diff --git a/lib/xsize.h b/lib/xsize.h
index 2d99a6b..34bd778 100644
--- a/lib/xsize.h
+++ b/lib/xsize.h
@@ -1,6 +1,6 @@
/* xsize.h -- Checked size_t computations.
- Copyright (C) 2003, 2008 Free Software Foundation, Inc.
+ Copyright (C) 2003, 2008, 2009 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
@@ -105,4 +105,19 @@ xmax (size_t size1, size_t size2)
#define size_in_bounds_p(SIZE) \
((SIZE) != SIZE_MAX)
Re: [PATCH] Fix $m4dirs count
On Sat, Nov 14, 2009 at 06:47:15AM -0700, Eric Blake wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Robert Millan on 11/9/2009 2:00 PM: This is wrong, since 'echo -n' is not portable. It should use printf instead, or find some other portable way to count $m4dirs. Hi, In that case, please use printf. It should work too. I was waiting for Bruno to reply, but your patch looks safe (with printf) and gnulib-tool already uses printf. So I'm pushing: Thanks. Any comments on my other patch? -- Robert Millan The DRM opt-in fallacy: Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all.
Re: [PATCH] fix build warning in fnmatch_loop.c
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Robert Millan on 11/9/2009 2:50 PM: I'm not a fan of unnecessary casts. Can't we instead write this as: : p - startp + 1U; to still show that we intend for unsigned math, but without a cast? The warning persists. Can you show the exact gcc version, command line options, and warning output that you are seeing with this construct? - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr+wZQACgkQ84KuGfSFAYD4KQCggrt+coE/xyY209QKLVyYFhzy g3wAoLR3MBXUcdUB2SAAbUa28B5ww6Ex =r6KP -END PGP SIGNATURE-
Re: xalloc.h drags in dependencies
Eric Blake wrote: I was surprised when I got a link failure when using the Solaris cc on test-chown.c due to missing program_name, even though using gcc did just fine. It turns out that mgetgroups.c uses xalloc_oversized from xalloc.h, which is only a macro and by itself drags nothing in; but the rest of xalloc.h includes four inline functions which in turn drag in references to xalloc_die, error, program_name, and thus the link failure. I thought about changing the #if HAVE_INLINE part of xalloc.h to instead be #if HAVE_INLINE defined __GNUC__, but this penalizes other compilers. I don't want to just open-code xalloc_oversized into mgetgroups.c. So my idea was to move the one portion of xalloc.h that does not depend on xalloc_die into a header that is already LGPL; that way, mgetgroups would depend only on xsize, not xalloc. So, how does this patch look? ... Subject: [PATCH 1/2] xalloc: move xalloc_oversized to xsize xalloc_oversized was the only thing in xalloc.h that did not depend on xalloc_die. But due to inline functions in xalloc.h, Solaris cc gives a link failure if you use only xalloc_oversized and don't provide xalloc_die. Moving it to the LGPL header xsize.h can allow other modules to break the link dependency. * modules/xalloc (Depends-on): Add xsize. * lib/xalloc.h (xalloc_oversized): Move... * lib/xsize.h: ...here. Sorry, but if you really want to move it, I'd much prefer to have it go to a completely new header file, than to require that people include xsize.h (with which I have a fundamental difference of opinion).
Re: [PATCH] fix build warning in fnmatch_loop.c
On Sat, Nov 14, 2009 at 07:41:24AM -0700, Eric Blake wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Robert Millan on 11/9/2009 2:50 PM: I'm not a fan of unnecessary casts. Can't we instead write this as: : p - startp + 1U; to still show that we intend for unsigned math, but without a cast? The warning persists. Can you show the exact gcc version, command line options, and warning output that you are seeing with this construct? gcc-4.4 -Ignulib -I./gnulib -I. -I./include -I./include -Wall -W -DGRUB_LIBDIR=\/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\ -g -O2 -DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64 -I./util/mkisofs/include -I./gnulib -Wno-all -Werror -MD -c -o grub_mkisofs-gnulib_fnmatch.o gnulib/fnmatch.c cc1: warnings being treated as errors In file included from gnulib/fnmatch.c:173: gnulib/fnmatch_loop.c: In function ‘ext_match’: gnulib/fnmatch_loop.c:1087: error: signed and unsigned type in conditional expression gnulib/fnmatch_loop.c:1095: error: signed and unsigned type in conditional expression This is GCC 4.4.2. With 4.3.2 I get the same result. -- Robert Millan The DRM opt-in fallacy: Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all.
Re: tests: temporary files may be left behind
Ralf Wildenhues wrote:
* Jim Meyering wrote on Sat, Nov 14, 2009 at 02:51:34PM CET:
Ralf Wildenhues wrote:
But that separate patch would belong before your proposed one, as the
zero trap exposes this issue more.
If someone actually objects, I'll change each exit N to (exit N); exit N
in the affected files, but I think it's not worthwhile, since few people
test on OSF1/Tru64, and even fewer use ash for /bin/sh.
Doesn't this hit you with zsh and with Solaris /bin/sh, too?
Yes, you're right. That constitutes too big a group, so...
Who would object to my making every test script source a file
that contains this function definition:
# We use a trap below for cleanup. This requires us to go through
# hoops to get the right exit status transported through the signal.
# So use `Exit STATUS' instead of `exit STATUS' inside of the tests.
# Turn off errexit here so that we don't trip the bug with OSF1/Tru64
# sh inside this function.
Exit ()
{
set +e
(exit $1)
exit $1
}
*Then* I'll make every test use Exit in place of most uses of exit.
Re: tests: temporary files may be left behind
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 11/14/2009 8:37 AM: Who would object to my making every test script source a file that contains this function definition: Making every gnulib unit test shell script source a common file seems reasonable to me. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr+0tMACgkQ84KuGfSFAYBD+QCcCQS4oh30XlwzfXnIZepO7lXz ChAAn03RlFhidAe6jvowDJsFtKwTzQC/ =79Sp -END PGP SIGNATURE-
Re: code coverage report for gnulib
Ralf Wildenhues ralf.wildenh...@gmx.de writes: Hi Simon, * Simon Josefsson wrote on Fri, Nov 13, 2009 at 10:41:20AM CET: I prepared a code coverage report for gnulib: http://www.gnu.org/software/gnulib/coverage/ Thank you for doing this! How did you create the report, and was manual hacking needed in order to get it to work? I'm running into some issues with lcov on packages using libtool and having sources in both the source and the build tree, requiring quite some manual symlinking so that they are all found. Yes, I had to create symlinks for some directories in particular the uni* stuff. I saved some of the commands below as notes for myself, so you can get the picture of what is needed. I think this is a bug in LCOV but working around it was easier than looking into fixing the bug... /Simon j...@mocca:~/000-gnulib-simple-0/gllib/unicase$ ln -s . unicase j...@mocca:~/000-gnulib-simple-0/gllib/unicase$ ln -s ../unictype j...@mocca:~/000-gnulib-simple-0/gllib/uniconv$ ln -s ../unistr.h . j...@mocca:~/000-gnulib-simple-0/gllib/uniconv$ ln -s . uniconv j...@mocca:~/000-gnulib-simple-0/gllib/unictype$ ln -s . unictype j...@mocca:~/000-gnulib-simple-0/gllib/unilbrk$ ln -s ../unistr.h . j...@mocca:~/000-gnulib-simple-0/gllib/unilbrk$ ln -s . unilbrk j...@mocca:~/000-gnulib-simple-0/gllib/unilbrk$ ln -s ../uniwidth j...@mocca:~/000-gnulib-simple-0/gllib/uniname$ ln -s . uniname j...@mocca:~/000-gnulib-simple-0/gllib/uninorm$ ln -s ../array-mergesort.h j...@mocca:~/000-gnulib-simple-0/gllib/uninorm$ ln -s ../unistr.h . j...@mocca:~/000-gnulib-simple-0/gllib/uninorm$ ln -s . uninorm j...@mocca:~/000-gnulib-simple-0/gllib/unistdio$ ln -s ../xsize.h j...@mocca:~/000-gnulib-simple-0/gllib/unistdio$ ln -s ../unistr.h . j...@mocca:~/000-gnulib-simple-0/gllib/unistdio$ ln -s ../printf-parse.c j...@mocca:~/000-gnulib-simple-0/gllib/unistdio$ ln -s ../vasnprintf.c j...@mocca:~/000-gnulib-simple-0/gllib/unistdio$ ln -s . unistdio j...@mocca:~/000-gnulib-simple-0/gllib/unistr$ ln -s ../unistr.h . j...@mocca:~/000-gnulib-simple-0/gllib/unistr$ ln -s . unistr j...@mocca:~/000-gnulib-simple-0/gllib/uniwbrk$ ln -s ../unistr.h . j...@mocca:~/000-gnulib-simple-0/gllib/uniwbrk$ ln -s . uniwbrk j...@mocca:~/000-gnulib-simple-0/gllib/uniwidth$ ln -s ../unistr.h . j...@mocca:~/000-gnulib-simple-0/gllib/uniwidth$ ln -s . uniwidth j...@mocca:~/000-gnulib-simple-0/gltests$ ln -s ../gllib j...@mocca:~/000-gnulib-simple-0/gltests/unicase$ ln -s . unicase j...@mocca:~/000-gnulib-simple-0/gltests/uniconv$ ln -s . uniconv j...@mocca:~/000-gnulib-simple-0/gltests/unictype$ ln -s . unictype j...@mocca:~/000-gnulib-simple-0/gltests/unilbrk$ ln -s . unilbrk
Error message doesn't say, error
Without some clear marker, it is more than a little hard to find. This is line 414 of 619 lines of typescript text: configure.ac:9: version `sharutils_version' doesn't follow Gnits standards that includes 140 lines of warning messages all caused by gnulib macros. By the way, I didn't find this: http://www.amath.washington.edu/~lf/tutorials/autoconf/gnits/gnits.html#SEC13 helpful to resolve the gnits problem. So, I haven't any idea what the gnitty complaint might be: m4_define([sharutils_version], [4.7.1]) Any help at all would be appreciated. The sharutils build has been broken for a couple of years now. Thanks! Regards, Bruce P.S. I will just remove gnits for now. It's too hard.
Re: Error message doesn't say, error
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruce Korb on 11/14/2009 11:01 AM: that includes 140 lines of warning messages all caused by gnulib We've brought that up before, but Bruno didn't like Ralf's patch to silence the warnings in his macros. For now, you'll just have to ignore warnings there. So, I haven't any idea what the gnitty complaint might be: m4_define([sharutils_version], [4.7.1]) Stable versions only need two levels of versions (major and minor), not three. P.S. I will just remove gnits for now. It's too hard. And underspecified. I, for one, won't fault you for not using gnits. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr+8T4ACgkQ84KuGfSFAYDEtQCeKU+A3u5JkDFVQ4yM3jClIRQW u54AoMsy9KuKxPZp91U+hnwE+9H8ap+v =V+69 -END PGP SIGNATURE-
Re: Error message doesn't say, error
Hi Eric, On Sat, Nov 14, 2009 at 10:04 AM, Eric Blake e...@byu.net wrote: According to Bruce Korb on 11/14/2009 11:01 AM: that includes 140 lines of warning messages all caused by gnulib We've brought that up before, but Bruno didn't like Ralf's patch to silence the warnings in his macros. For now, you'll just have to ignore warnings there. OK, I'll bring it up again then: THIS IS TOO HARD TO DEBUG WHEN THERE IS THAT MUCH CHAFF IN THE OUTPUT!!! If there is an emotional attachment to being able to see chaff, then check for VERBOSE=true in the environment. Forcing hapless coders to wade through hundreds of lines of meaningless chatter to try to find an error message that is not marked with ERROR: is simply way, way, way too much of a burden on anybody that does not live and breathe gnulib/autoconf stuff all day long. Sorry, meaning no offense, Bruno, I just vehemently hate pointless chaff. Thanks - Bruce
modules needed and not added into libgnu.a
Still trying to build shar (now without gnits). I see the sources for these in sharutils/lib, but: gcc -g -Wall -o shar shar.o encode.o ../lib/libgnu.a ../lib/libgnu.a shar.o: In function `walkdown': /home/gnu/proj/sharutils-bld/src/shar.c:407: undefined reference to `xmalloc' /home/gnu/proj/sharutils-bld/src/shar.c:414: undefined reference to `xmalloc' /home/gnu/proj/sharutils-bld/src/shar.c:426: undefined reference to `xrealloc' /home/gnu/proj/sharutils-bld/src/shar.c:461: undefined reference to `xrealloc' /home/gnu/proj/sharutils-bld/src/shar.c:469: undefined reference to `xrealloc' shar.o: In function `generate_mkdir': /home/gnu/proj/sharutils-bld/src/shar.c:720: undefined reference to `xrealloc' /home/gnu/proj/sharutils-bld/src/shar.c:723: undefined reference to `xmalloc' /home/gnu/proj/sharutils-bld/src/shar.c:728: undefined reference to `xstrdup' shar.o: In function `generate_one_header_line': /home/gnu/proj/sharutils-bld/src/shar.c:822: undefined reference to `offtostr' shar.o: In function `generate_full_header': /home/gnu/proj/sharutils-bld/src/shar.c:892: undefined reference to `xgetcwd' shar.o: In function `change_files': /home/gnu/proj/sharutils-bld/src/shar.c:1017: undefined reference to `xmalloc' shar.o: In function `shar': /home/gnu/proj/sharutils-bld/src/shar.c:1576: undefined reference to `md5_stream' shar.o: In function `parse_output_base_name': /home/gnu/proj/sharutils-bld/src/shar.c:1922: undefined reference to `xmalloc' /home/gnu/proj/sharutils-bld/src/shar.c:1926: undefined reference to `xmalloc' shar.o: In function `main': /home/gnu/proj/sharutils-bld/src/shar.c:2190: undefined reference to `get_submitter' /home/gnu/proj/sharutils-bld/src/shar.c:2218: undefined reference to `xmalloc' /home/gnu/proj/sharutils-bld/src/shar.c:2223: undefined reference to `xrealloc' /home/gnu/proj/sharutils-bld/src/shar.c:2227: undefined reference to `xstrdup' /home/gnu/proj/sharutils-bld/src/shar.c:2272: undefined reference to `xmalloc' collect2: ld returned 1 exit status
intprops.h needs multi-inclusion guard
In file included from ../lib/inttostr.h:24, from shar.c:140: ../lib/intprops.h:44:1: warning: TYPE_MINIMUM redefined shar.c:108:1: warning: this is the location of the previous definition ../lib/intprops.h:50:1: warning: TYPE_MAXIMUM redefined shar.c:113:1: warning: this is the location of the previous definition
Re: modules needed and not added into libgnu.a
Bruce Korb wrote:
Eric Blake wrote:
According to Bruce Korb on 11/14/2009 11:36 AM:
Still trying to build shar (now without gnits).
I see the sources for these in sharutils/lib, but:
Are you using gnulib-tool --import, the bootstrap script from gnulib, or
some other means to import gnulib modules? I'm wondering if it is just a
case of not importing enough modules.
That was part, but not all, of it:
Hi Eric,
I've solved it. No. Not true. I've kludged my way around it.
I now #include md5.c and whoami.c into shar.c and added
unlocked-io to my module list. It works, but it would be
better to compile those .c files into libgnu. How to do that
is not obvious enough from the gnulib-tool --list output.
Perhaps a ``gnulib-tool --apropos md5_stream get_submitter'' ??
I know what functions I want. I know what files they're in.
I just don't know which modules provide those functions or
cause those sources to get compiled.
Thanks for your help.
gnulib_libs=
error
inttostr
malloc
realloc
unlocked-io
xalloc
xgetcwd
xstrtoimax
${glib} --import ${gnulib_libs}
#ifndef __need_getopt not working in getopt.h
$ gcc -H -DHAVE_CONFIG_H -I. -I.. -g -Wall -MT close-hook.o \ -MD -MP -MF .deps/close-hook.Tpo -c -o close-hook.o close-hook.c . ../config.h .. ./stdlib.h ... /usr/include/stdlib.h /usr/include/features.h . /usr/include/sys/cdefs.h .. /usr/include/bits/wordsize.h . /usr/include/gnu/stubs.h .. /usr/include/bits/wordsize.h .. /usr/include/gnu/stubs-64.h /usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h /usr/include/bits/waitflags.h /usr/include/bits/waitstatus.h . /usr/include/endian.h .. /usr/include/bits/endian.h .. /usr/include/bits/byteswap.h ... /usr/include/bits/wordsize.h /usr/include/xlocale.h /usr/include/sys/types.h . /usr/include/bits/types.h .. /usr/include/bits/wordsize.h .. /usr/include/bits/typesizes.h . ./time.h .. /usr/include/time.h . /usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h . /usr/include/sys/select.h .. /usr/include/bits/select.h .. /usr/include/bits/sigset.h .. ./time.h ... /usr/include/time.h .. /usr/include/bits/time.h . /usr/include/sys/sysmacros.h . /usr/include/bits/pthreadtypes.h .. /usr/include/bits/wordsize.h /usr/include/alloca.h . /usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h ... /usr/lib64/gcc/x86_64-suse-linux/4.3/include/stddef.h .. ./error.h .. ./xstrtol.h ... ./getopt.h /usr/include/getopt.h In file included from ./xstrtol.h:22, from ../config.h:990, from close-hook.c:18: ./getopt.h:183: error: redefinition of 'struct option' I poked around a bit, but it is pretty tangled in there. All I know is that I get this failure building the lib directory. I'll try inserting this into config.h: # define _GETOPT_H 1 and force /usr/include/getopt.h to be skipped.
Re: #ifndef __need_getopt not working in getopt.h
This only seems to be happening during make check.
make check-recursive
make[2]: Entering directory `/home/gnu/proj/sharutils-bld/lib'
make[3]: Entering directory `/home/gnu/proj/sharutils-bld/lib'
rm -f getopt.h-t getopt.h \
{ echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
sed -e 's|@''HAVE_GETOPT_H''@|1|g' \
-e 's|@''INCLUDE_NEXT''@|include_next|g' \
-e 's|@''PRAGMA_SYSTEM_HEADER''@|#pragma GCC system_header|g' \
-e 's|@''NEXT_GETOPT_H''@|getopt.h|g' \
-e '/definition of GL_LINK_WARNING/r .././link-warning.h' \
./getopt.in.h; \
} getopt.h-t \
mv -f getopt.h-t getopt.h
gcc -DHAVE_CONFIG_H -I. -I.. -g -Wall -MT close-hook.o -MD -MP -MF
.deps/close-hook.Tpo -c -o close-hook.o close-hook.c
In file included from ./xstrtol.h:22,
from ../config.h:990,
from close-hook.c:18:
./getopt.h:183: error: redefinition of 'struct option'
make[3]: *** [close-hook.o] Error 1
make[3]: Leaving directory `/home/gnu/proj/sharutils-bld/lib'
make[2]: *** [check-recursive] Error 1
make[2]: Leaving directory `/home/gnu/proj/sharutils-bld/lib'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/home/gnu/proj/sharutils-bld/lib'
make: *** [check-recursive] Error 1
This means I got through it on the main build. On the repeat
build, it fails. Won't even do a plain make this time
through.
From earlier in the build:
rm -f libgnu.a
ar cru libgnu.a close-hook.o openat-die.o xalloc-die.o xstrtoimax.o \
basename-lgpl.o chdir-long.o dirname-lgpl.o dup-safer.o dup2.o exitfail.o \
fd-safer.o fdopendir.o getcwd.o imaxtostr.o offtostr.o openat-proc.o \
pipe-safer.o save-cwd.o stripslash.o uinttostr.o umaxtostr.o xgetcwd.o \
xmalloc.o xstrtol.o xstrtol-error.o xstrtoul.o
ranlib libgnu.a
make[4]: Leaving directory `/home/gnu/proj/sharutils-bld/lib'
Looks good. For anybody's possible amusement, attached is
the full diff between lib as it existed after the first build
and the current directory. getopt.h is quite different.
Why is this so hard? sharutils used to just compile.
Then all the m4 configury macros changed and it has been
hell every time I try to build the thing. It has not
been built for closing on 3 years now. I think I could do
this if I reverted all the auto* tools to the current set
from several years ago.
Extract from config.log:
configure:6169: checking getopt.h usability
configure:6169: gcc -c -g -Wall conftest.c 5
configure:6169: $? = 0
configure:6169: result: yes
configure:6169: checking getopt.h presence
configure:6169: gcc -E conftest.c
configure:6169: $? = 0
configure:6169: result: yes
configure:6169: checking for getopt.h
configure:6169: result: yes
Extract from lib/Makefile:
# We need the following in order to create getopt.h when the system
# doesn't have one that works with the given compiler.
getopt.h: getopt.in.h
$(AM_V_GEN)rm -f $...@-t $@ \
{ echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
sed -e 's|@''HAVE_GETOPT_H''@|$(HAVE_GETOPT_H)|g' \
-e 's|@''INCLUDE_NEXT''@|$(INCLUDE_NEXT)|g' \
-e 's|@''PRAGMA_SYSTEM_HEADER''@|#pragma GCC system_header|g' \
-e 's|@''NEXT_GETOPT_H''@|$(NEXT_GETOPT_H)|g' \
-e '/definition of GL_LINK_WARNING/r $(LINK_WARNING_H)' \
$(srcdir)/getopt.in.h; \
} $...@-t \
mv -f $...@-t $@
I'm running on linux. There is a /usr/include/getopt.h.
I bet it works with GCC.
Bruce Korb wrote:
$ gcc -H -DHAVE_CONFIG_H -I. -I.. -g -Wall -MT close-hook.o \
-MD -MP -MF .deps/close-hook.Tpo -c -o close-hook.o close-hook.c
.. ./xstrtol.h
... ./getopt.h
/usr/include/getopt.h
In file included from ./xstrtol.h:22,
from ../config.h:990,
from close-hook.c:18:
./getopt.h:183: error: redefinition of 'struct option'
I poked around a bit, but it is pretty tangled in there.
Only in lib/.deps: close-hook.Tpo
diff -ur lib/getopt.h lib-saved//getopt.h
--- lib/getopt.h 2009-11-14 12:45:08.0 -0800
+++ lib-saved//getopt.h 2005-06-07 14:51:19.0 -0700
@@ -1,103 +1,26 @@
-/* DO NOT EDIT! GENERATED AUTOMATICALLY! */
/* Declarations for getopt.
- Copyright (C) 1989-1994,1996-1999,2001,2003,2004,2005,2006,2007,2009
- Free Software Foundation, Inc.
+ Copyright (C) 1989-1994, 1996-1999, 2001 Free Software Foundation, Inc.
This file is part of the GNU C Library.
- 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 of the License, or
- (at your option) any later version.
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+
Re: intprops.h needs multi-inclusion guard
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruce Korb on 11/14/2009 12:15 PM: In file included from ../lib/inttostr.h:24, from shar.c:140: ../lib/intprops.h:44:1: warning: TYPE_MINIMUM redefined shar.c:108:1: warning: this is the location of the previous definition ../lib/intprops.h:50:1: warning: TYPE_MAXIMUM redefined shar.c:113:1: warning: this is the location of the previous definition A multi-inclusion guard won't help you with this error; your duplicate definition comes because shar.c is blindly redefining the macro with slightly different spelling rather than relying on intprops.h. But I don't see the harm in adding it, so I'm committing: - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr/MjIACgkQ84KuGfSFAYCiTgCgucFWaXf/pPpwee7YXURn4Wcc +SwAn2Ndo9tuByfGCC5matUzvDDhc2LR =T+i5 -END PGP SIGNATURE- From fdac57e87e1ba42f9ed516f0b6b828e4522719bb Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Sat, 14 Nov 2009 15:09:08 -0700 Subject: [PATCH] intprops: add double-inclusion guard * lib/intprops.h: Allow idempotent includes. Suggested by Bruce Korb. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |4 lib/intprops.h | 38 ++ 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5c8f1b6..4ff76e7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2009-11-14 Eric Blake e...@byu.net + intprops: add double-inclusion guard + * lib/intprops.h: Allow idempotent includes. + Suggested by Bruce Korb. + openat: detect Solaris fchownat bug * lib/fchownat.c (rpl_fchownat): Work around Solaris bug. Avoid penalizing glibc chownat when only lchownat is broken. diff --git a/lib/intprops.h b/lib/intprops.h index 002161e..325c397 100644 --- a/lib/intprops.h +++ b/lib/intprops.h @@ -1,6 +1,7 @@ /* intprops.h -- properties of integer types - Copyright (C) 2001, 2002, 2003, 2004, 2005 Free Software Foundation, Inc. + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2009 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 @@ -17,37 +18,40 @@ /* Written by Paul Eggert. */ -#include limits.h +#ifndef GL_INTPROPS_H +# define GL_INTPROPS_H + +# include limits.h /* The extra casts in the following macros work around compiler bugs, e.g., in Cray C 5.0.3.0. */ /* True if the arithmetic type T is an integer type. bool counts as an integer. */ -#define TYPE_IS_INTEGER(t) ((t) 1.5 == 1) +# define TYPE_IS_INTEGER(t) ((t) 1.5 == 1) /* True if negative values of the signed integer type T use two's complement, ones' complement, or signed magnitude representation, respectively. Much GNU code assumes two's complement, but some people like to be portable to all possible C hosts. */ -#define TYPE_TWOS_COMPLEMENT(t) ((t) ~ (t) 0 == (t) -1) -#define TYPE_ONES_COMPLEMENT(t) ((t) ~ (t) 0 == 0) -#define TYPE_SIGNED_MAGNITUDE(t) ((t) ~ (t) 0 (t) -1) +# define TYPE_TWOS_COMPLEMENT(t) ((t) ~ (t) 0 == (t) -1) +# define TYPE_ONES_COMPLEMENT(t) ((t) ~ (t) 0 == 0) +# define TYPE_SIGNED_MAGNITUDE(t) ((t) ~ (t) 0 (t) -1) /* True if the arithmetic type T is signed. */ -#define TYPE_SIGNED(t) (! ((t) 0 (t) -1)) +# define TYPE_SIGNED(t) (! ((t) 0 (t) -1)) /* The maximum and minimum values for the integer type T. These macros have undefined behavior if T is signed and has padding bits. If this is a problem for you, please let us know how to fix it for your host. */ -#define TYPE_MINIMUM(t) \ +# define TYPE_MINIMUM(t) \ ((t) (! TYPE_SIGNED (t) \ ? (t) 0 \ : TYPE_SIGNED_MAGNITUDE (t) \ ? ~ (t) 0 \ : ~ (t) 0 (sizeof (t) * CHAR_BIT - 1))) -#define TYPE_MAXIMUM(t) \ +# define TYPE_MAXIMUM(t) \ ((t) (! TYPE_SIGNED (t) \ ? (t) -1 \ : ~ (~ (t) 0 (sizeof (t) * CHAR_BIT - 1 @@ -58,20 +62,22 @@ tighter bound. Otherwise, it overestimates the true bound by one byte when applied to unsigned types of size 2, 4, 16, ... bytes. The symbol signed_type_or_expr__ is private to this header file. */ -#if __GNUC__ = 2 -# define signed_type_or_expr__(t) TYPE_SIGNED (__typeof__ (t)) -#else -# define signed_type_or_expr__(t) 1 -#endif +# if __GNUC__ = 2 +# define signed_type_or_expr__(t) TYPE_SIGNED (__typeof__ (t)) +# else +# define signed_type_or_expr__(t) 1 +# endif /* Bound on length of the string representing an integer type or expression T. Subtract 1 for the sign bit if T is signed; log10 (2.0) 146/485; add 1 for integer division truncation; add 1 more for a
Re: [PATCH] fix build warning in fnmatch_loop.c
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Robert Millan on 11/14/2009 8:30 AM:
: p - startp + 1U;
to still show that we intend for unsigned math, but without a cast?
The warning persists.
Can you show the exact gcc version, command line options, and warning
output that you are seeing with this construct?
gcc-4.4 -Ignulib -I./gnulib -I. -I./include -I./include -Wall -W
-DGRUB_LIBDIR=\/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\ -g -O2
-DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64 -I./util/mkisofs/include -I./gnulib
-Wno-all -Werror -MD -c -o grub_mkisofs-gnulib_fnmatch.o gnulib/fnmatch.c
cc1: warnings being treated as errors
In file included from gnulib/fnmatch.c:173:
gnulib/fnmatch_loop.c: In function ‘ext_match’:
gnulib/fnmatch_loop.c:1087: error: signed and unsigned type in conditional
expression
gnulib/fnmatch_loop.c:1095: error: signed and unsigned type in conditional
expression
This is GCC 4.4.2. With 4.3.2 I get the same result.
With 1U, I can't reproduce your warning with gcc 4.3.4 on a 32-bit machine:
$ cat foo.c
#include sys/types.h
#ifndef bar
# define bar 1
#endif
int
main (int argc, char **argv)
{
size_t plen;
size_t patt = 1;
char *p = abc, *startp = p + 1;
plen = argc 1 ? patt : p - startp + bar;
return !!argv[0];
}
$ gcc -Wall -W foo.c -o foo
foo.c: In function `main':
foo.c:11: warning: signed and unsigned type in conditional expression
$ gcc -Wall -W foo.c -o foo -Dbar=1U
$
Oh, I see. I repeated the experiment on a 64-bit machine, and got the
same error even with -Dbar=1U. It comes because (p - startp) is a 64-bit
ptrdiff_t, but 1U is only 32-bit, so it promotes to long rather than
unsigned long, leaving us in the same boat. But using -Dbar=1LU made the
warning go away. So I'm committing this:
- --
Don't work too hard, make some time for fun as well!
Eric Blake e...@byu.net
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkr/MjYACgkQ84KuGfSFAYCCVwCeKmRVO7UMNZizfmA+pzam9HRH
xDIAnA2x2uXQabcJSeKnuAnCAdwdI7h/
=hVRL
-END PGP SIGNATURE-
From a86ea846c82189cec7b2fad8679377d5475211c1 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Sat, 14 Nov 2009 15:25:49 -0700
Subject: [PATCH] fnmatch: avoid compiler warning
cond ? (size_t) : (char* - char* + 1) varies in signedness, but
using 1LU coerces the latter half to unsigned math.
* lib/fnmatch_loop.c (NEW_PATTERN): Coerce addition to unsigned,
to silence compiler warning about mismatch signedness in ?:.
Reported by Robert Millan.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |5 +
lib/fnmatch_loop.c |4 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 4ff76e7..a241c5b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2009-11-14 Eric Blake e...@byu.net
+ fnmatch: avoid compiler warning
+ * lib/fnmatch_loop.c (NEW_PATTERN): Coerce addition to unsigned,
+ to silence compiler warning about mismatch signedness in ?:.
+ Reported by Robert Millan.
+
intprops: add double-inclusion guard
* lib/intprops.h: Allow idempotent includes.
Suggested by Bruce Korb.
diff --git a/lib/fnmatch_loop.c b/lib/fnmatch_loop.c
index d1008c2..ad58bb2 100644
--- a/lib/fnmatch_loop.c
+++ b/lib/fnmatch_loop.c
@@ -1,4 +1,4 @@
-/* Copyright (C)
1991,1992,1993,1996,1997,1998,1999,2000,2001,2002,2003,2004,2005,2006
+/* Copyright (C)
1991,1992,1993,1996,1997,1998,1999,2000,2001,2002,2003,2004,2005,2006,2009
Free Software Foundation, Inc.
This file is part of the GNU C Library.
@@ -1071,7 +1071,7 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string,
const CHAR *string_end,
\
plen = (opt == L_('?') || opt == L_('@') \
? pattern_len \
- : p - startp + 1);\
+ : p - startp + 1UL); \
plensize = plen * sizeof (CHAR); \
newpsize = offsetof (struct patternlist, str) + plensize; \
if ((size_t) -1 / sizeof (CHAR) plen\
--
1.6.5.rc1
Re: #ifndef __need_getopt not working in getopt.h
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruce Korb on 11/14/2009 1:48 PM: rm -f libgnu.a ar cru libgnu.a close-hook.o openat-die.o xalloc-die.o xstrtoimax.o \ basename-lgpl.o chdir-long.o dirname-lgpl.o dup-safer.o dup2.o exitfail.o \ fd-safer.o fdopendir.o getcwd.o imaxtostr.o offtostr.o openat-proc.o \ pipe-safer.o save-cwd.o stripslash.o uinttostr.o umaxtostr.o xgetcwd.o \ xmalloc.o xstrtol.o xstrtol-error.o xstrtoul.o Is this something you can reproduce using just './gnulib-tool --with-tests - --test [your list of modules]'? Is this a case where you need to upgrade your list of modules to use getopt-posix or getopt-gnu, instead of the deprecated plain getopt? I'm running on linux. There is a /usr/include/getopt.h. Which kernel version, and glibc version? - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr/NekACgkQ84KuGfSFAYA81ACgxPLKN8mWYG+sJY0c9vV9ORyG BX4AniMxnAYdDV2zjjONtYC1wts95YD0 =XPd3 -END PGP SIGNATURE-
Re: [PATCH] test-freading: remove a temporary file
Hi Jim, Without this change, this test's temporary file is left behind every time it is run. Any objection? The #include and the first unlink are fine. Thanks, well spotted. The second unlink is better omitted, since when the message file operations failed message is printed, the user will likely want to inspect the contents of the file and fix the test. Bruno
Re: tests: temporary files may be left behind
Hi Jim,
I noticed that when the test-c-stack2.sh test is skipped,
it leaves behind its temporary file.
I propose to fix it by adding a trap ... 0.
I would prefer the attached patch, because it's simpler (straightforward)
and avoids the 'trap ... 0' command which
1. has portability problems, as Ralf pointed out,
2. forces a review of all calls to 'exit' in the same shell script.
A welcome side-effect is that with this change, you remove
temporary files from only one place: the trap, rather
than just prior to every other exit point.
For me, a very important property of the test files is that one can execute
them step by step, line by line, by copying the lines into an interactive bash.
Does your proposed patch introduce a behaviour change of test-atexit.sh
... test-uc_width2.sh (excluding test-c-stack2.sh)?
If no, then I object against the change, because it would be introducing
tricky code into files that already work perfectly fine.
If yes, i.e. if it causes the temporary files to be cleaned up in some border
cases that were not handled previously, then
- I find your suggestion good.
- However, I don't like the code duplication. So I would propose to have a
tests/cleanup.sh script that contains the necessary 'trap' commands,
together with a couple of comments, and modify the test-*.sh files only
through the addition of a single line:
. ${srcdir}/cleanup.sh
- In particular, I'm in favour of leaving the rm -rf $tmpfiles before the
final
exit statement in place, so that it is not forgotten when copying the lines
into an interactive bash, line by line.
Bruno
gnulib-patch1
Description: Binary data
