With these two change sets, ./configure --enable-gcc-warnings && make in builds with most warnings enabled also in lib/.
To make that work, I've chosen to disable-in-lib/ some warning options, via -Wno-missing-prototypes -Wno-uninitialized -Wno-unused-macros -Wno-old-style-definition" In addition, I've removed the offending "Unsigned_var < 0" tests from lib/reg*.c files, but added a few verify (! TYPE_SIGNED (Idx)); statements to ensure that the code doesn't compile if someone ever changes "Idx" to be a signed type. Bruno, I've patched isnan.c (no previous declaration of rpl_isnan) and unicodeio.c's fwrite_success_callback (ignored fwrite return value). IMHO, those two changes could easily go into gnulib. Regarding the regex and strftime changes, if others are interested in doing the same thing to their packages, and (like me) don't want to turn off the -W option that was evoking warnings, let me know and I'll push some variant of these patches to gnulib. The fewer gl/lib/*.diff files I have to carry in coreutils the better. I'd welcome a careful review of the reg*.c changes. >From e0301c0f177e37d8cdb0d72f7f382f1d27f00489 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Tue, 27 Oct 2009 12:12:11 +0100 Subject: [PATCH 1/2] build: allow whitespace violations in gl/lib/*.diff files * .gitattributes: Exempt gl/lib/*.diff. * .x-sc_prohibit_tab_based_indentation: Likewise. * .x-sc_space_tab:Likewise. --- .gitattributes | 2 ++ .x-sc_prohibit_tab_based_indentation | 1 + .x-sc_space_tab | 1 + 3 files changed, 4 insertions(+), 0 deletions(-) diff --git a/.gitattributes b/.gitattributes index 32f18fd..c3b2926 100644 --- a/.gitattributes +++ b/.gitattributes @@ -5,3 +5,5 @@ # # Derived from the regexp in emacs' lisp/add-log.el. # [diff "texinfo"] # funcname = "^...@node[ \t][ \t]*\\([^,][^,]*\\)" + +gl/lib/*.diff -whitespace diff --git a/.x-sc_prohibit_tab_based_indentation b/.x-sc_prohibit_tab_based_indentation index 2f5d921..388f94a 100644 --- a/.x-sc_prohibit_tab_based_indentation +++ b/.x-sc_prohibit_tab_based_indentation @@ -4,3 +4,4 @@ Makefile\.am$ ^tests/pr/ ChangeLog.* ^man/help2man$ +^gl/lib/.*\.c\.diff$ diff --git a/.x-sc_space_tab b/.x-sc_space_tab index f52ebd0..2ef3428 100644 --- a/.x-sc_space_tab +++ b/.x-sc_space_tab @@ -9,3 +9,4 @@ m4/lib-prefix.m4 m4/po.m4 aclocal.m4 src/c99-to-c89.diff +^gl/lib/.*\.c\.diff$ -- 1.6.5.2.344.ga473e >From 2239f2a53f2f4ee8051c736f25e33c13fbb0c99d Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Tue, 27 Oct 2009 12:06:43 +0100 Subject: [PATCH 2/2] build (--enable-gcc-warnings): enable gcc's -Werror also in lib/ * configure.ac (GNULIB_WARN_CFLAGS): Define. * lib/Makefile.am (AM_CFLAGS): Use $(GNULIB_WARN_CFLAGS) rather than $(WARN_CFLAGS) and add $(WERROR_CFLAGS). * gl/lib/isnan.c.diff: New file. * gl/lib/regcomp.c.diff: New file. * gl/lib/regex_internal.c.diff: New file. * gl/lib/regexec.c.diff: New file. * gl/lib/strftime.c.diff: New file. * gl/lib/unicodeio.c.diff: New file. --- configure.ac | 3 ++ gl/lib/isnan.c.diff | 13 ++++++++++++ gl/lib/regcomp.c.diff | 22 +++++++++++++++++++++ gl/lib/regex_internal.c.diff | 25 ++++++++++++++++++++++++ gl/lib/regexec.c.diff | 43 ++++++++++++++++++++++++++++++++++++++++++ gl/lib/strftime.c.diff | 21 ++++++++++++++++++++ gl/lib/unicodeio.c.diff | 21 ++++++++++++++++++++ lib/Makefile.am | 2 +- 8 files changed, 149 insertions(+), 1 deletions(-) create mode 100644 gl/lib/isnan.c.diff create mode 100644 gl/lib/regcomp.c.diff create mode 100644 gl/lib/regex_internal.c.diff create mode 100644 gl/lib/regexec.c.diff create mode 100644 gl/lib/strftime.c.diff create mode 100644 gl/lib/unicodeio.c.diff diff --git a/configure.ac b/configure.ac index 3efc819..f5c0959 100644 --- a/configure.ac +++ b/configure.ac @@ -114,6 +114,9 @@ if test "$gl_gcc_warnings" = yes; then AC_DEFINE([_FORTIFY_SOURCE], [2], [enable compile-time and run-time bounds-checking, and some warnings]) AC_DEFINE([GNULIB_PORTCHECK], [1], [enable some gnulib portability checks]) + + AC_SUBST([GNULIB_WARN_CFLAGS]) + GNULIB_WARN_CFLAGS="$WARN_CFLAGS -Wno-missing-prototypes -Wno-uninitialized -Wno-unused-macros -Wno-old-style-definition" fi AC_FUNC_FORK diff --git a/gl/lib/isnan.c.diff b/gl/lib/isnan.c.diff new file mode 100644 index 0000000..3602504 --- /dev/null +++ b/gl/lib/isnan.c.diff @@ -0,0 +1,13 @@ +diff --git a/lib/isnan.c b/lib/isnan.c +index a5ca38d..b818753 100644 +--- a/lib/isnan.c ++++ b/lib/isnan.c +@@ -67,6 +67,8 @@ + ((sizeof (DOUBLE) + sizeof (unsigned int) - 1) / sizeof (unsigned int)) + typedef union { DOUBLE value; unsigned int word[NWORDS]; } memory_double; + ++int FUNC (DOUBLE x); ++ + int + FUNC (DOUBLE x) + { diff --git a/gl/lib/regcomp.c.diff b/gl/lib/regcomp.c.diff new file mode 100644 index 0000000..1c0f2b6 --- /dev/null +++ b/gl/lib/regcomp.c.diff @@ -0,0 +1,22 @@ +diff --git a/lib/regcomp.c b/lib/regcomp.c +index 6472ff6..e03ae9e 100644 +--- a/lib/regcomp.c ++++ b/lib/regcomp.c +@@ -18,6 +18,7 @@ + with this program; if not, write to the Free Software Foundation, + Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ + ++#include "intprops.h" + static reg_errcode_t re_compile_internal (regex_t *preg, const char * pattern, + size_t length, reg_syntax_t syntax); + static void re_compile_fastmap_iter (regex_t *bufp, +@@ -2571,7 +2572,8 @@ parse_dup_op (bin_tree_t *elem, re_string_t *regexp, re_dfa_t *dfa, + /* This loop is actually executed only when end != REG_MISSING, + to rewrite <re>{0,n} as (<re>(<re>...<re>?)?)?... We have + already created the start+1-th copy. */ +- if ((Idx) -1 < 0 || end != REG_MISSING) ++ verify (! TYPE_SIGNED (Idx)); ++ if (end != REG_MISSING) + for (i = start + 2; i <= end; ++i) + { + elem = duplicate_tree (elem, dfa); diff --git a/gl/lib/regex_internal.c.diff b/gl/lib/regex_internal.c.diff new file mode 100644 index 0000000..2cede3c --- /dev/null +++ b/gl/lib/regex_internal.c.diff @@ -0,0 +1,25 @@ +diff --git a/lib/regex_internal.c b/lib/regex_internal.c +index 904b88e..61c8d9d 100644 +--- a/lib/regex_internal.c ++++ b/lib/regex_internal.c +@@ -18,6 +18,8 @@ + with this program; if not, write to the Free Software Foundation, + Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ + ++#include "verify.h" ++#include "intprops.h" + static void re_string_construct_common (const char *str, Idx len, + re_string_t *pstr, + RE_TRANSLATE_TYPE trans, bool icase, +@@ -1390,7 +1392,10 @@ static void + internal_function + re_node_set_remove_at (re_node_set *set, Idx idx) + { +- if (idx < 0 || idx >= set->nelem) ++ verify (! TYPE_SIGNED (Idx)); ++ /* if (idx < 0) ++ return; */ ++ if (idx >= set->nelem) + return; + --set->nelem; + for (; idx < set->nelem; idx++) diff --git a/gl/lib/regexec.c.diff b/gl/lib/regexec.c.diff new file mode 100644 index 0000000..2d1ab20 --- /dev/null +++ b/gl/lib/regexec.c.diff @@ -0,0 +1,43 @@ +diff --git a/lib/regexec.c b/lib/regexec.c +index 21a8166..dc04ac0 100644 +--- a/lib/regexec.c ++++ b/lib/regexec.c +@@ -18,6 +18,8 @@ + with this program; if not, write to the Free Software Foundation, + Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ + ++#include "verify.h" ++#include "intprops.h" + static reg_errcode_t match_ctx_init (re_match_context_t *cache, int eflags, + Idx n) internal_function; + static void match_ctx_clean (re_match_context_t *mctx) internal_function; +@@ -378,8 +380,9 @@ re_search_2_stub (struct re_pattern_buffer *bufp, + Idx len = length1 + length2; + char *s = NULL; + +- if (BE (length1 < 0 || length2 < 0 || stop < 0 || len < length1, 0)) +- return -2; ++ verify (! TYPE_SIGNED (Idx)); ++ /* if (BE (length1 < 0 || length2 < 0 || stop < 0 || len < length1, 0)) ++ return -2; */ + + /* Concatenate the strings. */ + if (length2 > 0) +@@ -431,11 +434,14 @@ re_search_stub (struct re_pattern_buffer *bufp, + Idx last_start = start + range; + + /* Check for out-of-range. */ +- if (BE (start < 0 || start > length, 0)) +- return -1; ++ verify (! TYPE_SIGNED (Idx)); ++ /* if (BE (start < 0, 0)) ++ return -1; */ ++ if (BE (start > length, 0)) ++ return -1; + if (BE (length < last_start || (0 <= range && last_start < start), 0)) + last_start = length; +- else if (BE (last_start < 0 || (range < 0 && start <= last_start), 0)) ++ else if (BE (/* last_start < 0 || */ (range < 0 && start <= last_start), 0)) + last_start = 0; + + __libc_lock_lock (dfa->lock); diff --git a/gl/lib/strftime.c.diff b/gl/lib/strftime.c.diff new file mode 100644 index 0000000..acd34f1 --- /dev/null +++ b/gl/lib/strftime.c.diff @@ -0,0 +1,21 @@ +diff --git a/lib/strftime.c b/lib/strftime.c +index fff5d38..712a35d 100644 +--- a/lib/strftime.c ++++ b/lib/strftime.c +@@ -31,6 +31,7 @@ + # else + # include "strftime.h" + # endif ++# include "ignore-value.h" + #endif + + #include <ctype.h> +@@ -203,7 +204,7 @@ extern char *tzname[]; + else if (to_uppcase) \ + fwrite_uppcase (p, (s), _n); \ + else \ +- fwrite ((s), _n, 1, p)) ++ ignore_value (fwrite ((s), _n, 1, p))) + #else + # define cpy(n, s) \ + add ((n), \ diff --git a/gl/lib/unicodeio.c.diff b/gl/lib/unicodeio.c.diff new file mode 100644 index 0000000..1d35e3c --- /dev/null +++ b/gl/lib/unicodeio.c.diff @@ -0,0 +1,21 @@ +diff --git a/lib/unicodeio.c b/lib/unicodeio.c +index 781621c..7a15b43 100644 +--- a/lib/unicodeio.c ++++ b/lib/unicodeio.c +@@ -38,6 +38,7 @@ + + #include "localcharset.h" + #include "unistr.h" ++#include "ignore-value.h" + + /* When we pass a Unicode character to iconv(), we must pass it in a + suitable encoding. The standardized Unicode encodings are +@@ -162,7 +163,7 @@ fwrite_success_callback (const char *buf, size_t buflen, void *callback_arg) + { + FILE *stream = (FILE *) callback_arg; + +- fwrite (buf, 1, buflen, stream); ++ ignore_value (fwrite (buf, 1, buflen, stream)); + return 0; + } + diff --git a/lib/Makefile.am b/lib/Makefile.am index 074cc9c..896458f 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -17,7 +17,7 @@ include gnulib.mk -AM_CFLAGS += $(WARN_CFLAGS) # $(WERROR_CFLAGS) +AM_CFLAGS += $(GNULIB_WARN_CFLAGS) $(WERROR_CFLAGS) libcoreutils_a_SOURCES += \ buffer-lcm.c buffer-lcm.h \ -- 1.6.5.2.344.ga473e