[libvirt] [PATCHv2 27/27] build: add syntax check for proper flags use

[Matthias Bolte]

2011/7/8 Eric Blake <eblake at redhat.com>:
> 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(-)

> @@ -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);

In 02/27 you added a syntax-check rule to enforce flags=%x. Does this
automatically cover oflags too, or does this need a change in the
rule?

> @@ -564,7 +564,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>         cmd = virCommandNewArgList(LIBEXECDIR "/libvirt_iohelper",
>                                    path,
>                                    NULL);
> -        virCommandAddArgFormat(cmd, "%d", flags);
> +        virCommandAddArgFormat(cmd, "%d", oflags);

In 02/27 you changed the printing of flags from %d to %x in debug
output. Maybe we should do that here too for consistence and adapt
libvirt_iohelper? Or is there any possibility for a version mismatch
between libvirt and libvirt_iohelper and we cannot change this anymore
without breaking backwards compatibility?

>         virCommandAddArgFormat(cmd, "%d", mode);

Same comment applies here about mode and switching from %d to %o.

ACK.

-- 
Matthias Bolte
http://photron.blogspot.com