[PATCH v2 1/2] util: Rework virFileFlock() to be unambiguous

[Martin Kletzander]

The boolean parameters for lock/unlock and read/write (shared/exclusive) caused
a lot of confusion when reading the callers.  The new approach is explicit and
unambiguous.

Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
Reviewed-by: Andrea Bolognani <abologna at redhat.com>
---
 src/util/virfile.c    | 27 ++++++++++++++++++---------
 src/util/virfile.h    |  9 ++++++++-
 src/util/virresctrl.c |  4 ++--
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 213acdbcaa2b..554011eecb25 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -465,23 +465,33 @@ int virFileUnlock(int fd, off_t start, off_t len)
 /**
  * virFileFlock:
  * @fd: file descriptor to call flock on
- * @lock: true for lock, false for unlock
- * @shared: true if shared, false for exclusive, ignored if `@lock == false`
+ * @op: operation to perform
  *
  * This is just a simple wrapper around flock(2) that errors out on unsupported
  * platforms.
  *
  * The lock will be released when @fd is closed or this function is called with
- * `@lock == false`.
+ * `@op == VIR_FILE_FLOCK_UNLOCK`.
  *
  * Returns 0 on success, -1 otherwise (with errno set)
  */
-int virFileFlock(int fd, bool lock, bool shared)
+int virFileFlock(int fd, virFileFlockOperation op)
 {
-    if (lock)
-        return flock(fd, shared ? LOCK_SH : LOCK_EX);
+    int flock_op = -1;
 
-    return flock(fd, LOCK_UN);
+    switch (op) {
+    case VIR_FILE_FLOCK_SHARED:
+        flock_op = LOCK_SH;
+        break;
+    case VIR_FILE_FLOCK_EXCLUSIVE:
+        flock_op = LOCK_EX;
+        break;
+    case VIR_FILE_FLOCK_UNLOCK:
+        flock_op = LOCK_UN;
+        break;
+    }
+
+    return flock(fd, flock_op);
 }
 
 #else /* WIN32 */
@@ -505,8 +515,7 @@ int virFileUnlock(int fd G_GNUC_UNUSED,
 
 
 int virFileFlock(int fd G_GNUC_UNUSED,
-                 bool lock G_GNUC_UNUSED,
-                 bool shared G_GNUC_UNUSED)
+                 virFileFlockOperation op G_GNUC_UNUSED)
 {
     errno = ENOSYS;
     return -1;
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 7a92364a5c9f..04428727fd3b 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -118,7 +118,14 @@ int virFileLock(int fd, bool shared, off_t start, off_t len, bool waitForLock)
 int virFileUnlock(int fd, off_t start, off_t len)
     G_GNUC_NO_INLINE;
 
-int virFileFlock(int fd, bool lock, bool shared);
+
+typedef enum {
+    VIR_FILE_FLOCK_EXCLUSIVE,
+    VIR_FILE_FLOCK_SHARED,
+    VIR_FILE_FLOCK_UNLOCK,
+} virFileFlockOperation;
+
+int virFileFlock(int fd, virFileFlockOperation op);
 
 typedef int (*virFileRewriteFunc)(int fd, const void *opaque);
 int virFileRewrite(const char *path,
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 3563fc560db5..b685b8bb6f7b 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -463,7 +463,7 @@ virResctrlLockWrite(void)
         return -1;
     }
 
-    if (virFileFlock(fd, true, true) < 0) {
+    if (virFileFlock(fd, VIR_FILE_FLOCK_SHARED) < 0) {
         virReportSystemError(errno, "%s", _("Cannot lock resctrl"));
         VIR_FORCE_CLOSE(fd);
         return -1;
@@ -485,7 +485,7 @@ virResctrlUnlock(int fd)
         virReportSystemError(errno, "%s", _("Cannot close resctrl"));
 
         /* Trying to save the already broken */
-        if (virFileFlock(fd, false, false) < 0)
+        if (virFileFlock(fd, VIR_FILE_FLOCK_UNLOCK) < 0)
             virReportSystemError(errno, "%s", _("Cannot unlock resctrl"));
 
         return -1;
-- 
2.27.0