From 761bbb17c7b7b3bf4a89e9f932c4340986aba278 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 7 Jul 2011 11:57:43 -0600 Subject: [PATCH] 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. --- cfg.mk | 30 ++++++++++++++++++++++++++---- src/fdstream.c | 28 ++++++++++++++-------------- src/fdstream.h | 6 +++--- src/locking/lock_driver_nop.c | 10 +++++----- src/locking/lock_manager.c | 7 ++++--- src/util/command.c | 2 +- src/util/iohelper.c | 18 +++++++++--------- src/util/util.c | 14 +++++++------- 8 files changed, 69 insertions(+), 46 deletions(-) diff --git a/cfg.mk b/cfg.mk index 7c3e2d2224..f98c94be64 100644 --- 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='\&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 4dbe4a3ef7..dd742e1980 100644 --- 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 a66902b69c..76f0cd030f 100644 --- 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 0dcd7941d8..4f35afa4cc 100644 --- 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 d27cf8fd92..f07f9d7d02 100644 --- 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 e2ece788d2..5bc6272e76 100644 --- 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 f519d5a7a8..0368eba7fc 100644 --- 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 0ca81df6de..910bb04be4 100644 --- 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")); -- 2.47.2