5:42 p.m.
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.
---
v3: minor tweaks to make syntax check rules catch a few more cases
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 7c3e2d2..60779eb 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='\<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 (acheived 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 4dbe4a3..dd742e1 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 a66902b..76f0cd0 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 0dcd794..4f35afa 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 d27cf8f..f07f9d7 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 f3c35ed..11dd7b0 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -622,7 +622,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 f519d5a..0368eba 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 0ca81df..910bb04 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"));
--
1.7.4.4