thr3ads.net - Libguestfs - Re: [Libguestfs] [PATCH 5/5] build: Enable some more warnings. [Nov 2015]

If this information is useful, please help other people find it:
Share via:

Richard W.M. Jones

2015-Nov-09 23:02 UTC

[Libguestfs] [PATCH 0/5] build: Enable some more warnings.

Add some warnings.  Well, the first patch is a miscellaneous
change, but patches 2-5 add some warnings.

Rich.

Richard W.M. Jones

2015-Nov-09 23:02 UTC

head link

[Libguestfs] [PATCH 1/5] gobject: Ignore gobject/docs/guestfs-scan.c.

I'm not sure what/why this file has suddenly appeared.  It may
be something to do with updating gtk-doc.

Whatever - ignore it.
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 0ca7d0d..0626fae 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -287,7 +287,7 @@ dist-hook:
 all-local:
 	cd $(srcdir); \
 	find $(DIST_SUBDIRS) -name '*.c' -o -name '*.pl' -o -name
'*.pm' | \
-	grep -v -E
'^(examples|gnulib|perl/(blib|examples)|po-docs|tests|test-data)/' | \
+	grep -v -E
'^(examples|gnulib|gobject/docs|perl/(blib|examples)|po-docs|tests|test-data)/'
| \
 	grep -v -E '/((guestfs|rc)_protocol\.c)$$' | \
 	grep -v -E '^python/utils.c$$' | \
 	LC_ALL=C sort > po/POTFILES
-- 
2.5.0

Richard W.M. Jones

2015-Nov-09 23:02 UTC

head link

[Libguestfs] [PATCH 2/5] Avoid gcc warnings about infinite loop.

This seemingly redundant change avoids a gcc warning/error:

  error: cannot optimize possibly infinite loops

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34114#c3 and
following for an explanation.

Of course gcc is completely wrong and stupid here, because there's no
possibility on current or future hardware that an array of size
SSIZE_MAX could be allocated since it would by definition occupy at
least all addressible memory (if it was an array of bytes, which this
isn't), leaving no room for the code that is being compiled.
---
 daemon/debug.c | 2 +-
 daemon/lvm.c   | 4 ++--
 daemon/md.c    | 2 +-
 daemon/xattr.c | 2 +-
 src/drives.c   | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/daemon/debug.c b/daemon/debug.c
index a210db1..abc7699 100644
--- a/daemon/debug.c
+++ b/daemon/debug.c
@@ -547,7 +547,7 @@ debug_progress (const char *subcmd, size_t argc, char *const
*const argv)
     uint64_t tsecs = secs * 10; /* 1/10ths of seconds */
     uint64_t i;
 
-    for (i = 1; i <= tsecs; ++i) {
+    for (i = 1; i < tsecs+1; ++i) {
       usleep (100000);
       notify_progress (i, tsecs);
     }
diff --git a/daemon/lvm.c b/daemon/lvm.c
index 421c1c5..d989986 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -215,7 +215,7 @@ do_vgcreate (const char *volgroup, char *const *physvols)
   argv[0] = str_lvm;
   argv[1] = "vgcreate";
   argv[2] = volgroup;
-  for (i = 3; i <= argc; ++i)
+  for (i = 3; i < argc+1; ++i)
     argv[i] = physvols[i-3];
 
   r = commandv (NULL, &err, (const char * const*) argv);
@@ -522,7 +522,7 @@ do_vg_activate (int activate, char *const *volgroups)
   argv[1] = "vgchange";
   argv[2] = "-a";
   argv[3] = activate ? "y" : "n";
-  for (i = 4; i <= argc; ++i)
+  for (i = 4; i < argc+1; ++i)
     argv[i] = volgroups[i-4];
 
   r = commandv (NULL, &err, (const char * const*) argv);
diff --git a/daemon/md.c b/daemon/md.c
index cf826c5..8bb4b54 100644
--- a/daemon/md.c
+++ b/daemon/md.c
@@ -467,7 +467,7 @@ parse_md_stat_line (char *line)
   return ret;
 
  error:
-  for (i = 0; i <= spaces; ++i) {
+  for (i = 0; i < spaces+1; ++i) {
     free (ret->guestfs_int_mdstat_list_val[i].mdstat_device);
     free (ret->guestfs_int_mdstat_list_val[i].mdstat_flags);
   }
diff --git a/daemon/xattr.c b/daemon/xattr.c
index c05124d..b599408 100644
--- a/daemon/xattr.c
+++ b/daemon/xattr.c
@@ -376,7 +376,7 @@ do_internal_lxattrlist (const char *path, char *const
*names)
      * entry[1..nr_attrs] are the attributes.
      */
     entry =
&ret->guestfs_int_xattr_list_val[ret->guestfs_int_xattr_list_len-nr_attrs-1];
-    for (m = 1; m <= nr_attrs; ++m) {
+    for (m = 1; m < nr_attrs+1; ++m) {
       entry[m].attrname = NULL;
       entry[m].attrval.attrval_len = 0;
       entry[m].attrval.attrval_val = NULL;
diff --git a/src/drives.c b/src/drives.c
index b9cc813..dc88495 100644
--- a/src/drives.c
+++ b/src/drives.c
@@ -524,7 +524,7 @@ add_drive_to_handle_at (guestfs_h *g, struct drive *d,
size_t drv_index)
   if (drv_index >= g->nr_drives) {
     g->drives = safe_realloc (g, g->drives,
                               sizeof (struct drive *) * (drv_index + 1));
-    while (g->nr_drives <= drv_index) {
+    while (g->nr_drives < drv_index+1) {
       g->drives[g->nr_drives] = NULL;
       g->nr_drives++;
     }
-- 
2.5.0

Richard W.M. Jones

2015-Nov-09 23:02 UTC

head link

[Libguestfs] [PATCH 3/5] Avoid various "declaration of <var> shadows a previous local"

I enabled the -Wshadow warning temporarily in order to do these fixes,
but had to disable it again afterwards.  The reason is that this warns
about shadowing globals, which is sort of a good thing, but because we
have a global called "verbose" just about everywhere, and at the same
time we baked a function argument called "verbose" into several
unchangable APIs, well, we're stuck without being able to use this
warning.
---
 daemon/augeas.c          | 12 ++++++------
 daemon/btrfs.c           |  2 +-
 daemon/mkfs.c            |  8 ++++----
 src/events.c             |  2 +-
 src/inspect-fs-windows.c |  1 -
 src/launch-direct.c      |  1 -
 src/launch-uml.c         |  1 -
 src/proto.c              |  6 +++---
 8 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/daemon/augeas.c b/daemon/augeas.c
index 59d3508..ea1163f 100644
--- a/daemon/augeas.c
+++ b/daemon/augeas.c
@@ -35,13 +35,13 @@
     if (code == AUG_ENOMEM)                                             \
       reply_with_error (fs ": augeas out of memory", ##__VA_ARGS__); 
\
     else {                                                              \
-      const char *message = aug_error_message (aug);                    \
-      const char *minor = aug_error_minor_message (aug);                \
-      const char *details = aug_error_details (aug);                    \
+      const char *aug_err_message = aug_error_message (aug);            \
+      const char *aug_err_minor = aug_error_minor_message (aug);        \
+      const char *aug_err_details = aug_error_details (aug);            \
       fprintf (stderr, fs ": %s%s%s%s%s", ##__VA_ARGS__,             
\
-	       message,							\
-	       minor ? ": " : "", minor ? minor : "",		
\
-	       details ? ": " : "", details ? details :
"");		\
+	       aug_err_message,                                         \
+	       aug_err_minor ? ": " : "", aug_err_minor ?
aug_err_minor : "", \
+	       aug_err_details ? ": " : "", aug_err_details ?
aug_err_details : ""); \
     }                                                                   \
   } while (0)
 
diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 652a17e..85dbe00 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -511,7 +511,7 @@ do_btrfs_subvolume_list (const mountable_t *fs)
     goto error;
   }
 
-  for (size_t i = 0; i < nr_subvolumes; ++i) {
+  for (i = 0; i < nr_subvolumes; ++i) {
     /* To avoid allocations, reuse the 'line' buffer to store the
      * path.  Thus we don't need to free 'line', since it will be
      * freed by the calling (XDR) code.
diff --git a/daemon/mkfs.c b/daemon/mkfs.c
index ee9e46d..dde35b2 100644
--- a/daemon/mkfs.c
+++ b/daemon/mkfs.c
@@ -117,14 +117,14 @@ do_mkfs (const char *fstype, const char *device, int
blocksize,
        * have to determine the block device sector size in order to do
        * this.
        */
-      int sectorsize = do_blockdev_getss (device);
-      if (sectorsize == -1)
+      int ss = do_blockdev_getss (device);
+      if (ss == -1)
         return -1;
 
-      int sectors_per_cluster = blocksize / sectorsize;
+      int sectors_per_cluster = blocksize / ss;
       if (sectors_per_cluster < 1 || sectors_per_cluster > 128) {
         reply_with_error ("unsupported cluster size for %s filesystem
(requested cluster size = %d, sector size = %d, trying sectors per cluster =
%d)",
-                          fstype, blocksize, sectorsize, sectors_per_cluster);
+                          fstype, blocksize, ss, sectors_per_cluster);
         return -1;
       }
 
diff --git a/src/events.c b/src/events.c
index 5fed0c0..2d065b8 100644
--- a/src/events.c
+++ b/src/events.c
@@ -132,7 +132,7 @@ guestfs_int_call_callbacks_message (guestfs_h *g, uint64_t
event,
        event == GUESTFS_EVENT_WARNING ||
        event == GUESTFS_EVENT_TRACE)) {
     bool from_appliance = event == GUESTFS_EVENT_APPLIANCE;
-    size_t i, i0;
+    size_t i0;
 
     /* APPLIANCE =>  <buf>
      * LIBRARY =>    libguestfs: <buf>\n
diff --git a/src/inspect-fs-windows.c b/src/inspect-fs-windows.c
index f8df5e5..af28bb7 100644
--- a/src/inspect-fs-windows.c
+++ b/src/inspect-fs-windows.c
@@ -451,7 +451,6 @@ check_windows_system_registry (guestfs_h *g, struct
inspect_fs *fs)
       /* Get the binary value.  Is it a fixed disk? */
       CLEANUP_FREE char *blob = NULL;
       char *device;
-      size_t len;
       int64_t type;
 
       type = guestfs_hivex_value_type (g, v);
diff --git a/src/launch-direct.c b/src/launch-direct.c
index 7540c19..29ec359 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -718,7 +718,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   if (g->recovery_proc) {
     r = fork ();
     if (r == 0) {
-      int i;
       struct sigaction sa;
       pid_t qemu_pid = data->pid;
       pid_t parent_pid = getppid ();
diff --git a/src/launch-uml.c b/src/launch-uml.c
index 2ec7022..5bd0ce7 100644
--- a/src/launch-uml.c
+++ b/src/launch-uml.c
@@ -350,7 +350,6 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)
   if (g->recovery_proc) {
     r = fork ();
     if (r == 0) {
-      int i;
       struct sigaction sa;
       pid_t vmlinux_pid = data->pid;
       pid_t parent_pid = getppid ();
diff --git a/src/proto.c b/src/proto.c
index efe9dfb..cb884bf 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -174,14 +174,14 @@ check_daemon_socket (guestfs_h *g)
 
   /* Read and process progress messages that happen during FileIn. */
   if (flag == GUESTFS_PROGRESS_FLAG) {
-    char buf[PROGRESS_MESSAGE_SIZE];
+    char mbuf[PROGRESS_MESSAGE_SIZE];
     guestfs_progress message;
 
-    n = g->conn->ops->read_data (g, g->conn, buf,
PROGRESS_MESSAGE_SIZE);
+    n = g->conn->ops->read_data (g, g->conn, mbuf,
PROGRESS_MESSAGE_SIZE);
     if (n <= 0) /* 0 or -1 */
       return n;
 
-    xdrmem_create (&xdr, buf, PROGRESS_MESSAGE_SIZE, XDR_DECODE);
+    xdrmem_create (&xdr, mbuf, PROGRESS_MESSAGE_SIZE, XDR_DECODE);
     xdr_guestfs_progress (&xdr, &message);
     xdr_destroy (&xdr);
 
-- 
2.5.0

Richard W.M. Jones

2015-Nov-09 23:02 UTC

head link

[Libguestfs] [PATCH 4/5] Avoid "function 'asprintf_nowarn' can never be inlined because it uses variable argument lists".

Move this to a separate compilation unit and don't inline it.
---
 daemon/Makefile.am |  1 +
 daemon/daemon.h    | 24 +-----------------------
 daemon/format.c    | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 po/POTFILES        |  1 +
 4 files changed, 52 insertions(+), 23 deletions(-)
 create mode 100644 daemon/format.c

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 1a23cda..8055235 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -118,6 +118,7 @@ guestfsd_SOURCES = \
 	findfs.c \
 	fill.c \
 	find.c \
+	format.c \
 	fs-min-size.c \
 	fsck.c \
 	fstrim.c \
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 820e9cb..7fbb2a2 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -154,29 +154,7 @@ extern int random_name (char *template);
 
 extern char *get_random_uuid (void);
 
-/* This just stops gcc from giving a warning about our custom printf
- * formatters %Q and %R.  See guestfs-hacking(1) for more
- * info about these.  In GCC 4.8.0 the warning is even harder to
- * 'trick', hence the need for the #pragma directives.
- */
-#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >=
4.8.0 */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
-#endif
-static inline int
-asprintf_nowarn (char **strp, const char *fmt, ...)
-{
-  int r;
-  va_list args;
-
-  va_start (args, fmt);
-  r = vasprintf (strp, fmt, args);
-  va_end (args);
-  return r;
-}
-#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >=
4.8.0 */
-#pragma GCC diagnostic pop
-#endif
+extern int asprintf_nowarn (char **strp, const char *fmt, ...);
 
 /* Use by the CLEANUP_* macros. */
 extern void cleanup_free (void *ptr);
diff --git a/daemon/format.c b/daemon/format.c
new file mode 100644
index 0000000..2ccc24b
--- /dev/null
+++ b/daemon/format.c
@@ -0,0 +1,49 @@
+/* libguestfs - the guestfsd daemon
+ * Copyright (C) 2010-2015 Red Hat 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
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "daemon.h"
+
+/* This just stops gcc from giving a warning about our custom printf
+ * formatters %Q and %R.  See guestfs-hacking(1) for more
+ * info about these.  In GCC 4.8.0 the warning is even harder to
+ * 'trick', hence the need for the #pragma directives.
+ */
+#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >=
4.8.0 */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
+#endif
+int
+asprintf_nowarn (char **strp, const char *fmt, ...)
+{
+  int r;
+  va_list args;
+
+  va_start (args, fmt);
+  r = vasprintf (strp, fmt, args);
+  va_end (args);
+  return r;
+}
+#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >=
4.8.0 */
+#pragma GCC diagnostic pop
+#endif
diff --git a/po/POTFILES b/po/POTFILES
index d87d668..09a8425 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -49,6 +49,7 @@ daemon/file.c
 daemon/fill.c
 daemon/find.c
 daemon/findfs.c
+daemon/format.c
 daemon/fs-min-size.c
 daemon/fsck.c
 daemon/fstrim.c
-- 
2.5.0

Richard W.M. Jones

2015-Nov-09 23:02 UTC

head link

[Libguestfs] [PATCH 5/5] build: Enable some more warnings.

After fixing some warnings (see prior commits), we can now enable
them in configure.ac.
---
 m4/guestfs_c.m4 | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/m4/guestfs_c.m4 b/m4/guestfs_c.m4
index e91446f..7250294 100644
--- a/m4/guestfs_c.m4
+++ b/m4/guestfs_c.m4
@@ -43,22 +43,15 @@ fi
 dnl This, $nw, is the list of warnings we disable.
 nw 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"           # Not an error
-nw="$nw -Wtraditional-conversion"    # Don't care about pre-ANSI
compilers
 nw="$nw -Wpadded"                    # Our structs are not padded
-nw="$nw -Wvla"                       # two warnings in mount.c
-dnl 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
+nw="$nw -Wvla"                       # Allow variable length arrays.
+nw="$nw -Wshadow"                    # Not useful, as it applies to
global vars
 nw="$nw -Wunsafe-loop-optimizations" # just a warning that an
optimization
                                      # was not possible, safe to ignore
+dnl things I might fix soon:
 nw="$nw -Wpacked"                    # Allow attribute((packed)) on
structs
 nw="$nw -Wlong-long"                 # Allow long long since it's
required
                                      # by Python, Ruby and xstrtoll.
-- 
2.5.0

Pino Toscano

2015-Nov-18 10:19 UTC

head link

Re: [Libguestfs] [PATCH 5/5] build: Enable some more warnings.

On Monday 09 November 2015 23:02:15 Richard W.M. Jones
wrote:> After fixing some warnings (see prior commits), we can now enable
> them in configure.ac.
> ---
> [...]
> -nw="$nw -Winline"                    # daemon.h's
asprintf_nowarn
Disabling -Winline causes the Python binding code to fail with build
with GCC < 4.9, because of the static inline functions in
python/guestfs-py.h. See for example on CentOS 7.1 and openSUSE 13.1:
https://ci.centos.org/view/libguestfs/job/libguestfs/label=libguestfs-ci-CentOS-7-slave01/108/console
https://ci.centos.org/view/libguestfs/job/libguestfs/label=libguestfs-ci-opensuse-13.2-slave01/108/console

Would it be acceptable to de-inline them into normal functions?

-- 
Pino Toscano

Reasonably Related Threads

Search for more apparently analagous threads

Libguestfs - Nov 2015 - Re: [PATCH 5/5] build: Enable some more warnings.

[Libguestfs] [PATCH 0/5] build: Enable some more warnings.

[Libguestfs] [PATCH 1/5] gobject: Ignore gobject/docs/guestfs-scan.c.

[Libguestfs] [PATCH 2/5] Avoid gcc warnings about infinite loop.

[Libguestfs] [PATCH 3/5] Avoid various "declaration of <var> shadows a previous local"

[Libguestfs] [PATCH 4/5] Avoid "function 'asprintf_nowarn' can never be inlined because it uses variable argument lists".

[Libguestfs] [PATCH 5/5] build: Enable some more warnings.

Re: [Libguestfs] [PATCH 5/5] build: Enable some more warnings.

Reasonably Related Threads