bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

enable -Werror for lib/ in coreutils


From: Jim Meyering
Subject: enable -Werror for lib/ in coreutils
Date: Tue, 27 Oct 2009 13:24:27 +0100

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 <address@hidden>
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 = "address@hidden \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 <address@hidden>
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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]