build: add syntax check for proper flags use

Enforce the recent flags cleanups - we want to use 'unsigned int flags'
in any of our APIs (except where backwards compatibility is important,
in the public migration APIs), and that all flags are checked for
validity (except when there are stub functions that completely
ignore the flags argument).

There are a few minor tweaks done here to avoid false positives:
signed arguments passed to open() are renamed oflags, and flags
arguments that are legitimately ignored are renamed flags_unused.

* cfg.mk (sc_flags_usage): New rule.
(exclude_file_name_regexp--sc_flags_usage): And a few exemptions.
(sc_flags_debug): Tweak wording.
* src/util/iohelper.c (runIO, main): Rename variable.
* src/util/util.c (virSetInherit): Likewise.
* src/fdstream.h (virFDStreamOpenFile, virFDStreamCreateFile):
Likewise.
* src/fdstream.c (virFDStreamOpenFileInternal)
(virFDStreamOpenFile, virFDStreamCreateFile): Likewise.
* src/util/command.c (virExecWithHook) [WIN32]: Likewise.
* src/util/util.c (virFileOpenAs, virDirCreate) [WIN32]: Likewise.
* src/locking/lock_manager.c (virLockManagerPluginNew)
[!HAVE_DLFCN_H]: Likewise.
* src/locking/lock_driver_nop.c (virLockManagerNopNew)
(virLockManagerNopAddResource, virLockManagerNopAcquire)
(virLockManagerNopRelease, virLockManagerNopInquire): Likewise.

diff --git a/cfg.mk b/cfg.mk
index 7c3e2d2224f68a278b128c54c6706a07fb7e2261..f98c94be6467f409015dd67abc24cc6008dc1cc2 100644 (file)
--- a/cfg.mk
+++ b/cfg.mk
@@ -273,10 +273,32 @@ sc_avoid_write:
 # In debug statements, print flags as bitmask and mode_t as octal.
 sc_flags_debug:
        @prohibit='\<mode=%[0-9.]*[diux]'                               \
-       halt='debug mode_t values with %o'                              \
+       halt='use %o to debug mode_t values'                            \
          $(_sc_search_regexp)
-       @prohibit='\<flags=%[0-9.]*l*[diou]'                            \
-       halt='debug flag values with %x'                                \
+       @prohibit='[Ff]lags=%[0-9.]*l*[diou]'                           \
+       halt='use %x to debug flag values'                              \
+         $(_sc_search_regexp)
+
+# Prefer 'unsigned int flags', along with checks for unknown flags.
+# For historical reasons, we are stuck with 'unsigned long flags' in
+# migration, so check for those known 4 instances and no more in public
+# API.  Also check that no flags are marked unused, and 'unsigned' should
+# appear before any declaration of a flags variable (achieved by
+# prohibiting the word prior to the type from ending in anything other
+# than d).  The existence of long long, and of documentation about
+# flags, makes the regex in the third test slightly harder.
+sc_flags_usage:
+       @test "$$(cat $(srcdir)/include/libvirt/libvirt.h.in            \
+           $(srcdir)/include/libvirt/virterror.h                       \
+           $(srcdir)/include/libvirt/libvirt-qemu.h                    \
+         | grep -c '\(long\|unsigned\) flags')" != 4 &&                \
+         { echo '$(ME): new API should use "unsigned int flags"' 1>&2; \
+           exit 1; } || :
+       @prohibit=' flags ''ATTRIBUTE_UNUSED'                           \
+       halt='flags should be checked with virCheckFlags'               \
+         $(_sc_search_regexp)
+       @prohibit='^[^@]*([^d] (int|long long)|[^dg] long) flags[;,)]'  \
+       halt='flags should be unsigned'                                 \
          $(_sc_search_regexp)
 
 # Avoid functions that can lead to double-close bugs.
@@ -645,7 +667,7 @@ exclude_file_name_regexp--sc_avoid_write = \
 
 exclude_file_name_regexp--sc_bindtextdomain = ^(tests|examples)/
 
-exclude_file_name_regexp--sc_flags_debug = ^docs/
+exclude_file_name_regexp--sc_flags_usage = ^docs/
 
 exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \
   ^src/rpc/gendispatch\.pl$$

diff --git a/src/fdstream.c b/src/fdstream.c
index 4dbe4a3ef7eae9a77b459802469ed35533fa6c80..dd742e1980d33ff21f731baa8f36657f601169f1 100644 (file)
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -504,7 +504,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
                             const char *path,
                             unsigned long long offset,
                             unsigned long long length,
-                            int flags,
+                            int oflags,
                             int mode,
                             bool delete)
 {
@@ -514,13 +514,13 @@ virFDStreamOpenFileInternal(virStreamPtr st,
     virCommandPtr cmd = NULL;
     int errfd = -1;
 
-    VIR_DEBUG("st=%p path=%s flags=%x offset=%llu length=%llu mode=%o delete=%d",
-              st, path, flags, offset, length, mode, delete);
+    VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o delete=%d",
+              st, path, oflags, offset, length, mode, delete);
 
-    if (flags & O_CREAT)
-        fd = open(path, flags, mode);
+    if (oflags & O_CREAT)
+        fd = open(path, oflags, mode);
     else
-        fd = open(path, flags);
+        fd = open(path, oflags);
     if (fd < 0) {
         virReportSystemError(errno,
                              _("Unable to open stream for '%s'"),
@@ -545,7 +545,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
          !S_ISFIFO(sb.st_mode))) {
         int childfd;
 
-        if ((flags & O_RDWR) == O_RDWR) {
+        if ((oflags & O_RDWR) == O_RDWR) {
             streamsReportError(VIR_ERR_INTERNAL_ERROR,
                                _("%s: Cannot request read and write flags together"),
                                path);
@@ -562,7 +562,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
         cmd = virCommandNewArgList(LIBEXECDIR "/libvirt_iohelper",
                                    path,
                                    NULL);
-        virCommandAddArgFormat(cmd, "%d", flags);
+        virCommandAddArgFormat(cmd, "%d", oflags);
         virCommandAddArgFormat(cmd, "%d", mode);
         virCommandAddArgFormat(cmd, "%llu", offset);
         virCommandAddArgFormat(cmd, "%llu", length);
@@ -575,7 +575,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
          */
         delete = false;
 
-        if (flags == O_RDONLY) {
+        if (oflags == O_RDONLY) {
             childfd = fds[1];
             fd = fds[0];
             virCommandSetOutputFD(cmd, &childfd);
@@ -620,10 +620,10 @@ int virFDStreamOpenFile(virStreamPtr st,
                         const char *path,
                         unsigned long long offset,
                         unsigned long long length,
-                        int flags,
+                        int oflags,
                         bool delete)
 {
-    if (flags & O_CREAT) {
+    if (oflags & O_CREAT) {
         streamsReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Attempt to create %s without specifying mode"),
                            path);
@@ -631,19 +631,19 @@ int virFDStreamOpenFile(virStreamPtr st,
     }
     return virFDStreamOpenFileInternal(st, path,
                                        offset, length,
-                                       flags, 0, delete);
+                                       oflags, 0, delete);
 }
 
 int virFDStreamCreateFile(virStreamPtr st,
                           const char *path,
                           unsigned long long offset,
                           unsigned long long length,
-                          int flags,
+                          int oflags,
                           mode_t mode,
                           bool delete)
 {
     return virFDStreamOpenFileInternal(st, path,
                                        offset, length,
-                                       flags | O_CREAT,
+                                       oflags | O_CREAT,
                                        mode, delete);
 }

diff --git a/src/fdstream.h b/src/fdstream.h
index a66902b69c9a356a9ceb774864c4fe003838ed2a..76f0cd030fbf04edbea1bc74ad67ed39a72dcb97 100644 (file)
--- a/src/fdstream.h
+++ b/src/fdstream.h
@@ -1,7 +1,7 @@
 /*
  * fdstream.h: generic streams impl for file descriptors
  *
- * Copyright (C) 2009-2010 Red Hat, Inc.
+ * Copyright (C) 2009-2011 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -37,13 +37,13 @@ int virFDStreamOpenFile(virStreamPtr st,
                         const char *path,
                         unsigned long long offset,
                         unsigned long long length,
-                        int flags,
+                        int oflags,
                         bool delete);
 int virFDStreamCreateFile(virStreamPtr st,
                           const char *path,
                           unsigned long long offset,
                           unsigned long long length,
-                          int flags,
+                          int oflags,
                           mode_t mode,
                           bool delete);
 

diff --git a/src/locking/lock_driver_nop.c b/src/locking/lock_driver_nop.c
index 0dcd7941d88e9614da79e5110f6e99e78dc89429..4f35afa4ccdcc15a78a7b87d902aa0b121458503 100644 (file)
--- a/src/locking/lock_driver_nop.c
+++ b/src/locking/lock_driver_nop.c
@@ -49,7 +49,7 @@ static int virLockManagerNopNew(virLockManagerPtr lock ATTRIBUTE_UNUSED,
                                 unsigned int type ATTRIBUTE_UNUSED,
                                 size_t nparams ATTRIBUTE_UNUSED,
                                 virLockManagerParamPtr params ATTRIBUTE_UNUSED,
-                                unsigned int flags ATTRIBUTE_UNUSED)
+                                unsigned int flags_unused ATTRIBUTE_UNUSED)
 {
     return 0;
 }
@@ -59,7 +59,7 @@ static int virLockManagerNopAddResource(virLockManagerPtr lock ATTRIBUTE_UNUSED,
                                         const char *name ATTRIBUTE_UNUSED,
                                         size_t nparams ATTRIBUTE_UNUSED,
                                         virLockManagerParamPtr params ATTRIBUTE_UNUSED,
-                                        unsigned int flags ATTRIBUTE_UNUSED)
+                                        unsigned int flags_unused ATTRIBUTE_UNUSED)
 {
 
     return 0;
@@ -68,7 +68,7 @@ static int virLockManagerNopAddResource(virLockManagerPtr lock ATTRIBUTE_UNUSED,
 
 static int virLockManagerNopAcquire(virLockManagerPtr lock ATTRIBUTE_UNUSED,
                                     const char *state ATTRIBUTE_UNUSED,
-                                    unsigned int flags ATTRIBUTE_UNUSED,
+                                    unsigned int flags_unused ATTRIBUTE_UNUSED,
                                     int *fd ATTRIBUTE_UNUSED)
 {
     return 0;
@@ -76,7 +76,7 @@ static int virLockManagerNopAcquire(virLockManagerPtr lock ATTRIBUTE_UNUSED,
 
 static int virLockManagerNopRelease(virLockManagerPtr lock ATTRIBUTE_UNUSED,
                                     char **state,
-                                    unsigned int flags ATTRIBUTE_UNUSED)
+                                    unsigned int flags_unused ATTRIBUTE_UNUSED)
 {
     if (state)
         *state = NULL;
@@ -86,7 +86,7 @@ static int virLockManagerNopRelease(virLockManagerPtr lock ATTRIBUTE_UNUSED,
 
 static int virLockManagerNopInquire(virLockManagerPtr lock ATTRIBUTE_UNUSED,
                                     char **state,
-                                    unsigned int flags ATTRIBUTE_UNUSED)
+                                    unsigned int flags_unused ATTRIBUTE_UNUSED)
 {
     if (state)
         *state = NULL;

diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c
index d27cf8fd921fa187e1eef92d010f7d8bd39e5c3d..f07f9d7d02058cb41d911c627ef67492e1446218 100644 (file)
--- a/src/locking/lock_manager.c
+++ b/src/locking/lock_manager.c
@@ -190,9 +190,10 @@ cleanup:
     return NULL;
 }
 #else /* !HAVE_DLFCN_H */
-virLockManagerPluginPtr virLockManagerPluginNew(const char *name ATTRIBUTE_UNUSED,
-                                                const char *configFile ATTRIBUTE_UNUSED,
-                                                unsigned int flags ATTRIBUTE_UNUSED)
+virLockManagerPluginPtr
+virLockManagerPluginNew(const char *name ATTRIBUTE_UNUSED,
+                        const char *configFile ATTRIBUTE_UNUSED,
+                        unsigned int flags_unused ATTRIBUTE_UNUSED)
 {
     virLockError(VIR_ERR_INTERNAL_ERROR, "%s",
                  _("this platform is missing dlopen"));

diff --git a/src/util/command.c b/src/util/command.c
index e2ece788d20028d26926acf062a63e2f52e8322f..5bc6272e7620dccbc8dac271f872dde8beb348c7 100644 (file)
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -619,7 +619,7 @@ virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED,
                 int infd ATTRIBUTE_UNUSED,
                 int *outfd ATTRIBUTE_UNUSED,
                 int *errfd ATTRIBUTE_UNUSED,
-                int flags ATTRIBUTE_UNUSED,
+                int flags_unused ATTRIBUTE_UNUSED,
                 virExecHook hook ATTRIBUTE_UNUSED,
                 void *data ATTRIBUTE_UNUSED,
                 char *pidfile ATTRIBUTE_UNUSED)

diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index f519d5a7a8e364e8d64eccb0c79f1576cd6e0052..0368eba7fcf72d2421b940c865630d6c836054c4 100644 (file)
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -43,7 +43,7 @@
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
 static int runIO(const char *path,
-                 int flags,
+                 int oflags,
                  int mode,
                  unsigned long long offset,
                  unsigned long long length)
@@ -56,10 +56,10 @@ static int runIO(const char *path,
     const char *fdinname, *fdoutname;
     unsigned long long total = 0;
 
-    if (flags & O_CREAT) {
-        fd = open(path, flags, mode);
+    if (oflags & O_CREAT) {
+        fd = open(path, oflags, mode);
     } else {
-        fd = open(path, flags);
+        fd = open(path, oflags);
     }
     if (fd < 0) {
         virReportSystemError(errno, _("Unable to open %s"), path);
@@ -79,7 +79,7 @@ static int runIO(const char *path,
         goto cleanup;
     }
 
-    switch (flags & O_ACCMODE) {
+    switch (oflags & O_ACCMODE) {
     case O_RDONLY:
         fdin = fd;
         fdinname = path;
@@ -97,7 +97,7 @@ static int runIO(const char *path,
     default:
         virReportSystemError(EINVAL,
                              _("Unable to process file with flags %d"),
-                             (flags & O_ACCMODE));
+                             (oflags & O_ACCMODE));
         goto cleanup;
     }
 
@@ -144,7 +144,7 @@ int main(int argc, char **argv)
     virErrorPtr err;
     unsigned long long offset;
     unsigned long long length;
-    int flags;
+    int oflags;
     int mode;
     unsigned int delete;
 
@@ -169,7 +169,7 @@ int main(int argc, char **argv)
 
     path = argv[1];
 
-    if (virStrToLong_i(argv[2], NULL, 10, &flags) < 0) {
+    if (virStrToLong_i(argv[2], NULL, 10, &oflags) < 0) {
         fprintf(stderr, _("%s: malformed file flags %s"), argv[0], argv[2]);
         exit(EXIT_FAILURE);
     }
@@ -192,7 +192,7 @@ int main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }
 
-    if (runIO(path, flags, mode, offset, length) < 0)
+    if (runIO(path, oflags, mode, offset, length) < 0)
         goto error;
 
     if (delete)

diff --git a/src/util/util.c b/src/util/util.c
index 0ca81df6de53d0f9131fbaac2d29d105dabf53dd..910bb04be4d428f4fa64acdf29a6c6ad1f7bec84 100644 (file)
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -252,14 +252,14 @@ virArgvToString(const char *const *argv)
 #ifndef WIN32
 
 int virSetInherit(int fd, bool inherit) {
-    int flags;
-    if ((flags = fcntl(fd, F_GETFD)) < 0)
+    int fflags;
+    if ((fflags = fcntl(fd, F_GETFD)) < 0)
         return -1;
     if (inherit)
-        flags &= ~FD_CLOEXEC;
+        fflags &= ~FD_CLOEXEC;
     else
-        flags |= FD_CLOEXEC;
-    if ((fcntl(fd, F_SETFD, flags)) < 0)
+        fflags |= FD_CLOEXEC;
+    if ((fcntl(fd, F_SETFD, fflags)) < 0)
         return -1;
     return 0;
 }
@@ -981,7 +981,7 @@ int virFileOpenAs(const char *path ATTRIBUTE_UNUSED,
                   mode_t mode ATTRIBUTE_UNUSED,
                   uid_t uid ATTRIBUTE_UNUSED,
                   gid_t gid ATTRIBUTE_UNUSED,
-                  unsigned int flags ATTRIBUTE_UNUSED)
+                  unsigned int flags_unused ATTRIBUTE_UNUSED)
 {
     virUtilError(VIR_ERR_INTERNAL_ERROR,
                  "%s", _("virFileOpenAs is not implemented for WIN32"));
@@ -993,7 +993,7 @@ int virDirCreate(const char *path ATTRIBUTE_UNUSED,
                  mode_t mode ATTRIBUTE_UNUSED,
                  uid_t uid ATTRIBUTE_UNUSED,
                  gid_t gid ATTRIBUTE_UNUSED,
-                 unsigned int flags ATTRIBUTE_UNUSED)
+                 unsigned int flags_unused ATTRIBUTE_UNUSED)
 {
     virUtilError(VIR_ERR_INTERNAL_ERROR,
                  "%s", _("virDirCreate is not implemented for WIN32"));