Libguestfs - Aug 2009 - src/ is now warning-free, too

These patches first make src/ warning free, and then
turn on the strict warning options.

   75 0001-build-suppress-an-ignored-write-return-value-warning.patch
   38 0002-build-suppress-an-ignored-dup-return-value-warning.patch
   27 0003-generator.ml-suppress-signed-unsigned-compare-warnin.patch
   48 0004-build-don-t-perform-arithmetic-on-void-pointers.patch
   30 0005-suppress-signed-unsigned-comparison-warnings.patch
   28 0006-suppress-a-warning-from-Wswitch-default.patch
   36 0007-suppress-warnings-from-Wmissing-noreturn.patch
  102 0008-build-new-configure-time-option-enable-gcc-warnings.patch

$ diffstat 000*
 b/bootstrap        |    1
 b/configure.ac     |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 b/src/Makefile.am  |    4 ++-
 b/src/generator.ml |    3 --
 b/src/guestfs.c    |    4 +--
 src/Makefile.am    |    5 ++--
 src/guestfs.c      |   32 +++++++++++++++++----------
 7 files changed, 91 insertions(+), 19 deletions(-)

From: Jim Meyering <meyering at redhat.com>

* bootstrap (modules): Add ignore-value.
* src/guestfs.c: Include "ignore-value.h".
(stdout_event): Ignore failure to write to stderr.
Also, prefer STDERR_FILENO over the literal "2".
* src/Makefile.am (libguestfs_la_CPPFLAGS): Include gnulib's .h files.
(libprotocol_la_CFLAGS): Remove -Wall -Wno-unused.
---
 bootstrap       |    1 +
 src/Makefile.am |    4 +++-
 src/guestfs.c   |    3 ++-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/bootstrap b/bootstrap
index cea8c40..76e2216 100755
--- a/bootstrap
+++ b/bootstrap
@@ -53,6 +53,7 @@ modules='
 gitlog-to-changelog
 gnu-make
 gnumakefile
+ignore-value
 maintainer-makefile
 manywarnings
 warnings
diff --git a/src/Makefile.am b/src/Makefile.am
index 9eb71b9..8d03a77 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -54,7 +54,7 @@ libprotocol_la_SOURCES = \
   guestfs_protocol.c \
   guestfs_protocol.h

-libprotocol_la_CFLAGS = -Wall -Wno-unused
+libprotocol_la_CFLAGS 
 $(BUILT_SOURCES): stamp-generator

@@ -117,6 +117,8 @@ libguestfs_la_LIBADD = libprotocol.la

 libguestfs_la_CFLAGS = -Wall
-DGUESTFS_DEFAULT_PATH='"$(libdir)/guestfs"'

+libguestfs_la_CPPFLAGS = -I$(top_srcdir)/gnulib/lib
+
 if HAVE_RPCGEN
 guestfs_protocol.c: guestfs_protocol.x
 	rm -f $@-t $@-t2
diff --git a/src/guestfs.c b/src/guestfs.c
index ad3980f..ecdf9e5 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -59,6 +59,7 @@

 #include "guestfs.h"
 #include "guestfs_protocol.h"
+#include "ignore-value.h"

 #ifdef HAVE_GETTEXT
 #include "gettext.h"
@@ -1697,7 +1698,7 @@ stdout_event (struct guestfs_main_loop *ml, guestfs_h *g,
void *data,

   /* In verbose mode, copy all log messages to stderr. */
   if (g->verbose)
-    write (2, buf, n);
+    ignore_value (write (STDERR_FILENO, buf, n));

   /* It's an actual log message, send it upwards if anyone is listening. */
   if (g->log_message_cb)
-- 
1.6.4.378.g88f2f

From: Jim Meyering <meyering at redhat.com>

* src/guestfs.c (guestfs_launch): Handle dup failure.
---
 src/guestfs.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/guestfs.c b/src/guestfs.c
index ecdf9e5..1cd4f9e 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -1162,11 +1162,18 @@ guestfs_launch (guestfs_h *g)
     close (1);
     close (wfd[1]);
     close (rfd[0]);
-    dup (wfd[0]);
-    dup (rfd[1]);
+
+    int fail = 0;
+    fail |= dup (wfd[0]);
+    fail |= dup (rfd[1]);
     close (wfd[0]);
     close (rfd[1]);

+    if (fail) {
+      perror ("dup failed");
+      _exit (1);
+    }
+
 #if 0
     /* Set up a new process group, so we can signal this process
      * and all subprocesses (eg. if qemu is really a shell script).
-- 
1.6.4.378.g88f2f

From: Jim Meyering <meyering at redhat.com>

* src/generator.ml (check_reply_header): Emit parameter declarations
that are unsigned, so as to avoid signed/unsigned-compare warnings.
---
 src/generator.ml |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/generator.ml b/src/generator.ml
index cc97dd5..ddc9ab9 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -4532,7 +4532,7 @@ and generate_client_actions ()  static int
 check_reply_header (guestfs_h *g,
                     const struct guestfs_message_header *hdr,
-                    int proc_nr, int serial)
+                    unsigned int proc_nr, unsigned int serial)
 {
   if (hdr->prog != GUESTFS_PROGRAM) {
     error (g, \"wrong program (%%d/%%d)\", hdr->prog,
GUESTFS_PROGRAM);
-- 
1.6.4.378.g88f2f

From: Jim Meyering <meyering at redhat.com>

* src/guestfs.c (receive_file_data_sync, xread, xwrite): Use char*.
---
 src/guestfs.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/guestfs.c b/src/guestfs.c
index 1cd4f9e..6eae692 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -540,8 +540,9 @@ guestfs_safe_memdup (guestfs_h *g, void *ptr, size_t size)
 }

 static int
-xwrite (int fd, const void *buf, size_t len)
+xwrite (int fd, const void *v_buf, size_t len)
 {
+  const char *buf = v_buf;
   int r;

   while (len > 0) {
@@ -557,8 +558,9 @@ xwrite (int fd, const void *buf, size_t len)
 }

 static int
-xread (int fd, void *buf, size_t len)
+xread (int fd, void *v_buf, size_t len)
 {
+  char *buf = v_buf;
   int r;

   while (len > 0) {
@@ -2515,7 +2517,7 @@ receive_file_data_sync (guestfs_h *g, void **buf, size_t
*len_r)

     if (buf) {
       *buf = safe_realloc (g, *buf, len + ctx.chunks[i].data.data_len);
-      memcpy (*buf+len, ctx.chunks[i].data.data_val,
+      memcpy (((char *)*buf)+len, ctx.chunks[i].data.data_val,
               ctx.chunks[i].data.data_len);
     }
     len += ctx.chunks[i].data.data_len;
-- 
1.6.4.378.g88f2f

From: Jim Meyering <meyering at redhat.com>

* src/guestfs.c [struct guestfs_h] (msg_in_size, msg_in_allocated):
(msg_out_size, msg_out_allocated): Change type from int to unsigned int.
---
 src/guestfs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/guestfs.c b/src/guestfs.c
index 6eae692..0c007d2 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -202,9 +202,9 @@ struct guestfs_h

   /* Messages sent and received from the daemon. */
   char *msg_in;
-  int msg_in_size, msg_in_allocated;
+  unsigned int msg_in_size, msg_in_allocated;
   char *msg_out;
-  int msg_out_size, msg_out_pos;
+  unsigned int msg_out_size, msg_out_pos;

   int msg_next_serial;
 };
-- 
1.6.4.378.g88f2f

From: Jim Meyering <meyering at redhat.com>

* src/guestfs.c (guestfs_end_busy): Add a "default:" label.
---
 src/guestfs.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/guestfs.c b/src/guestfs.c
index 0c007d2..8491205 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -1628,8 +1628,10 @@ guestfs_end_busy (guestfs_h *g)
     case CONFIG:
     case READY:
       break;
+
     case LAUNCHING:
     case NO_HANDLE:
+    default:
       error (g, _("guestfs_end_busy: called when in state %d"),
g->state);
       return -1;
     }
-- 
1.6.4.378.g88f2f

From: Jim Meyering <meyering at redhat.com>

Even though these functions are marked as "not implemented yet",
and they will surely return a value once implemented, ...
* src/guestfs.c (select_add_timeout): Declare with noreturn attribute.
(select_remove_timeout): Likewise.
---
 src/guestfs.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/guestfs.c b/src/guestfs.c
index 8491205..0d54d65 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -2625,6 +2625,7 @@ select_remove_handle (guestfs_main_loop *mlv, guestfs_h
*g, int fd)
 }

 static int
+__attribute__((noreturn))
 select_add_timeout (guestfs_main_loop *mlv, guestfs_h *g, int interval,
                     guestfs_handle_timeout_cb cb, void *data)
 {
@@ -2634,6 +2635,7 @@ select_add_timeout (guestfs_main_loop *mlv, guestfs_h *g,
int interval,
 }

 static int
+__attribute__((noreturn))
 select_remove_timeout (guestfs_main_loop *mlv, guestfs_h *g, int timer)
 {
   //struct select_main_loop *ml = (struct select_main_loop *) mlv;
-- 
1.6.4.378.g88f2f

From: Jim Meyering <meyering at redhat.com>

* configure.ac: Define/configure it.
* src/Makefile.am: Use new variables.
---
 configure.ac    |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/Makefile.am |    4 ++-
 2 files changed, 64 insertions(+), 1 deletions(-)

diff --git a/configure.ac b/configure.ac
index d74fffb..fc2f457 100644
--- a/configure.ac
+++ b/configure.ac
@@ -47,6 +47,67 @@ AC_PROG_CPP
 gl_EARLY
 gl_INIT

+AC_ARG_ENABLE([gcc-warnings],
+  [AS_HELP_STRING([--enable-gcc-warnings],
+                  [turn on lots of GCC warnings (for developers)])],
+  [case $enableval in
+     yes|no) ;;
+     *)      AC_MSG_ERROR([bad value $enableval for gcc-warnings option]) ;;
+   esac
+   gl_gcc_warnings=$enableval],
+  [gl_gcc_warnings=no]
+)
+
+if test "$gl_gcc_warnings" = yes; then
+  gl_WARN_ADD([-Werror], [WERROR_CFLAGS])
+  AC_SUBST([WERROR_CFLAGS])
+
+  nw+  # This, $nw, is the list of warnings we disable.
+  nw="$nw -Wdeclaration-after-statement" # too useful to forbid
+  nw="$nw -Waggregate-return"       # anachronistic
+  nw="$nw -Wc++-compat"             # We don't care about C++
compilers
+  nw="$nw -Wundef"                  # Warns on '#if
GNULIB_FOO' etc in gnulib
+  nw="$nw -Wtraditional"            # Warns on #elif which we use
often
+  nw="$nw -Wcast-qual"              # Too many warnings for now
+  nw="$nw -Wconversion"             # Too many warnings for now
+  nw="$nw -Wsystem-headers"         # Don't let system headers
trigger warnings
+  nw="$nw -Wsign-conversion"        # Too many warnings for now
+  nw="$nw -Wtraditional-conversion" # Too many warnings for now
+  nw="$nw -Wunreachable-code"       # Too many warnings for now
+  nw="$nw -Wpadded"                 # Our structs are not padded
+  nw="$nw -Wredundant-decls"        # openat.h declares e.g., mkdirat
+  nw="$nw -Wlogical-op"             # any use of fwrite provokes this
+  nw="$nw -Wvla"                    # two warnings in mount.c
+  # things I might fix soon:
+  nw="$nw -Wmissing-format-attribute" # daemon.h's
asprintf_nowarn
+  nw="$nw -Winline"                 # daemon.h's asprintf_nowarn
+  nw="$nw -Wshadow"                 # numerous, plus we're not
unanimous
+  # ?? -Wstrict-overflow
+
+  gl_MANYWARN_ALL_GCC([ws])
+  gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw])
+  for w in $ws; do
+    gl_WARN_ADD([$w])
+  done
+  gl_WARN_ADD([-Wno-unused-parameter]) # stubs.c
+  gl_WARN_ADD([-Wno-jump-misses-init]) # stubs.c
+  gl_WARN_ADD([-Wno-unused-variable]) # FIXME: only temporary, for
guestfs_protocol.c, etc
+
+  # In spite of excluding -Wlogical-op above, it is enabled, as of
+  # gcc 4.5.0 20090517, and it provokes warnings in cat.c, dd.c, truncate.c
+  gl_WARN_ADD([-Wno-logical-op])
+
+  gl_WARN_ADD([-fdiagnostics-show-option])
+
+  AC_SUBST([WARN_CFLAGS])
+
+  AC_DEFINE([lint], [1], [Define to 1 if the compiler is checking for lint.])
+  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])
+fi
+
 AC_C_PROTOTYPES
 test "x$U" != "x" && AC_MSG_ERROR([Compiler not
ANSI compliant])

diff --git a/src/Makefile.am b/src/Makefile.am
index 8d03a77..e38c336 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -115,7 +115,9 @@ libguestfs_la_SOURCES = \
 noinst_LTLIBRARIES = libprotocol.la
 libguestfs_la_LIBADD = libprotocol.la

-libguestfs_la_CFLAGS = -Wall
-DGUESTFS_DEFAULT_PATH='"$(libdir)/guestfs"'
+libguestfs_la_CFLAGS = \
+  -DGUESTFS_DEFAULT_PATH='"$(libdir)/guestfs"' \
+  $(WARN_CFLAGS) $(WERROR_CFLAGS)

 libguestfs_la_CPPFLAGS = -I$(top_srcdir)/gnulib/lib

-- 
1.6.4.378.g88f2f