2:26 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.
* 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 | 21 ++++++++++++++++++++-
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, 63 insertions(+), 43 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 24539b1..a9ee9ea 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -278,6 +278,25 @@ sc_flags_debug:
halt='debug flag values with %x' \
$(_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 and in a few other places.
+# Three tests in this check: a fixed number of non-int flags in public
+# API, no flags marked unused, and 'unsigned' should appear before any
+# declaration of a flags variable (hence the prohibit line of [^d]).
+# The existence of long long makes the third test slightly harder.
+sc_flags_usage:
+ @test "$$(grep -c 'long flags' \
+ $(srcdir)/include/libvirt/libvirt.h.in)" != 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.
sc_prohibit_close:
@prohibit='([^>.]|^)\<[fp]?close *\(' \
@@ -632,7 +651,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 c9fe2f3..73545c7 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -505,7 +505,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
const char *path,
unsigned long long offset,
unsigned long long length,
- int flags,
+ int oflags,
int mode,
bool delete)
{
@@ -516,13 +516,13 @@ virFDStreamOpenFileInternal(virStreamPtr st,
int errfd = -1;
pid_t pid = 0;
- 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'"),
@@ -547,7 +547,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);
@@ -564,7 +564,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);
@@ -577,7 +577,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
*/
delete = false;
- if (flags == O_RDONLY) {
+ if (oflags == O_RDONLY) {
childfd = fds[1];
fd = fds[0];
virCommandSetOutputFD(cmd, &childfd);
@@ -626,10 +626,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);
@@ -637,19 +637,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 6c19cd1..8b01bcb 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -617,7 +617,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 2165f26..4bd37ad 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;
}
@@ -997,7 +997,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"));
@@ -1009,7 +1009,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