[PATCH 3/3] tools/libxl: Rework invariants in libxl__domain_get_device_model_uid()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Various version of gcc, when compiling with -Og, complain:

  libxl_dm.c: In function 'libxl__domain_get_device_model_uid':
  libxl_dm.c:256:12: error: 'kill_by_uid' may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
    256 |         if (kill_by_uid)
        |            ^

The logic is very tangled.

Two paths unconditionally set user before checking for associated errors.
This interferes with the expected use of uninitialised-variable heuristics to
force compile failures for ill-formed exit paths.

Use b_info->device_model_user and LIBXL_QEMU_USER_SHARED as appliable, and
only set user on the goto out paths.

All goto out paths now are comprised of the form:
  user = NULL;
  rc = 0;

or:
  user = non-NULL;
  indended_uid = ...;
  kill_by_uid = ...;
  rc = 0;

As a consequence, indended_uid can drop its default of -1, the dm_restrict can
drop its now-stale "just in case" comment and the redundant setting of
kill_by_uid to work around this issue at other optimisation levels.

Finally, rewrite the comment about invariants, indicating the split between
the out and err lables, and associated rc values.  Additionally, reword the
"is {not,} set" terminology to "is {non-,}NULL" to be more precise.

No funcational change.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Ian Jackson <iwj@xxxxxxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
---
 tools/libs/light/libxl_dm.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 7843c283ca..8a7e084d89 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -127,7 +127,7 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
     struct passwd *user_base, user_pwbuf;
     int rc;
     char *user;
-    uid_t intended_uid = -1;
+    uid_t intended_uid;
     bool kill_by_uid;
 
     /* Only qemu-upstream can run as a different uid */
@@ -135,33 +135,34 @@ static int libxl__domain_get_device_model_uid(libxl__gc 
*gc,
         return 0;
 
     /*
-     * From this point onward, all paths should go through the `out`
-     * label.  The invariants should be:
+     * From this point onward, all paths should go through the `out` (success,
+     * rc = 0) or `err` (failure, rc != 0) labels.  The invariants should be:
      * - rc may be 0, or an error code.
-     * - if rc is an error code, user and intended_uid are ignored.
-     * - if rc is 0, user may be set or not set.
-     * - if user is set, then intended_uid must be set to a UID matching
+     * - if rc is an error code, all settings are ignored.
+     * - if rc is 0, user may be NULL or non-NULL.
+     * - if user is non-NULL, then intended_uid must be set to a UID matching
      *   the username `user`, and kill_by_uid must be set to the appropriate
      *   value.  intended_uid will be checked for root (0).
      */
-    
+
     /*
      * If device_model_user is present, set `-runas` even if
      * dm_restrict isn't in use
      */
-    user = b_info->device_model_user;
-    if (user) {
-        rc = userlookup_helper_getpwnam(gc, user, &user_pwbuf, &user_base);
+    if (b_info->device_model_user) {
+        rc = userlookup_helper_getpwnam(gc, b_info->device_model_user,
+                                        &user_pwbuf, &user_base);
         if (rc)
             goto err;
 
         if (!user_base) {
             LOGD(ERROR, guest_domid, "Couldn't find device_model_user %s",
-                 user);
+                 b_info->device_model_user);
             rc = ERROR_INVAL;
             goto err;
         }
 
+        user = b_info->device_model_user;
         intended_uid = user_base->pw_uid;
         kill_by_uid = true;
         rc = 0;
@@ -175,8 +176,7 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
     if (!libxl_defbool_val(b_info->dm_restrict)) {
         LOGD(DEBUG, guest_domid,
              "dm_restrict disabled, starting QEMU as root");
-        user = NULL; /* Should already be null, but just in case */
-        kill_by_uid = false; /* Keep older versions of gcc happy */
+        user = NULL;
         rc = 0;
         goto out;
     }
@@ -219,13 +219,14 @@ static int libxl__domain_get_device_model_uid(libxl__gc 
*gc,
      * QEMU_USER_SHARED.  NB for QEMU_USER_SHARED, all QEMU will run
      * as the same UID, we can't kill by uid; therefore don't set uid.
      */
-    user = LIBXL_QEMU_USER_SHARED;
-    rc = userlookup_helper_getpwnam(gc, user, &user_pwbuf, &user_base);
+    rc = userlookup_helper_getpwnam(gc, LIBXL_QEMU_USER_SHARED,
+                                    &user_pwbuf, &user_base);
     if (rc)
         goto err;
     if (user_base) {
         LOGD(WARN, guest_domid, "Could not find user %s, falling back to %s",
              LIBXL_QEMU_USER_RANGE_BASE, LIBXL_QEMU_USER_SHARED);
+        user = LIBXL_QEMU_USER_SHARED;
         intended_uid = user_base->pw_uid;
         kill_by_uid = false;
         rc = 0;
-- 
2.11.0