[Libguestfs] [PATCH 1/3] inspection: Refactor windows systemroot detection to allow re-use

[Matthew Booth]

This change refactors guestfs___has_windows_systemroot to
guestfs___get_windows_systemroot. The new function returns a
dynamically allocated char * which must be freed.

The new function is no less efficient than before, as it returns the
result of guestfs___case_sensitive_path_silently, which is required
anyway. The new code is slightly more efficient than before, as it
re-uses the result of this testing in guestfs___check_windows_root
rather than running it again.
---
 src/guestfs-internal.h   |  4 +--
 src/inspect-fs-windows.c | 72 +++++++++++++++++++++++-------------------------
 src/inspect-fs.c         |  8 ++++--
 3 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 6e97948..bc13b3c 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -668,8 +668,8 @@ extern int guestfs___check_hurd_root (guestfs_h *g, struct inspect_fs *fs);
 
 /* inspect-fs-windows.c */
 extern char *guestfs___case_sensitive_path_silently (guestfs_h *g, const char *);
-extern int guestfs___has_windows_systemroot (guestfs_h *g);
-extern int guestfs___check_windows_root (guestfs_h *g, struct inspect_fs *fs);
+extern char * guestfs___get_windows_systemroot (guestfs_h *g);
+extern int guestfs___check_windows_root (guestfs_h *g, struct inspect_fs *fs, char *windows_systemroot);
 
 /* inspect-fs-cd.c */
 extern int guestfs___check_installer_root (guestfs_h *g, struct inspect_fs *fs);
diff --git a/src/inspect-fs-windows.c b/src/inspect-fs-windows.c
index a979775..8ddea95 100644
--- a/src/inspect-fs-windows.c
+++ b/src/inspect-fs-windows.c
@@ -101,60 +101,58 @@ static char *map_registry_disk_blob (guestfs_h *g, const void *blob);
  * "/Program Files" and "/System Volume Information".  Those would
  * *not* be Windows root disks.  (RHBZ#674130)
  */
-static const char *systemroots[] =
-  { "/windows", "/winnt", "/win32", "/win", NULL };
 
-int
-guestfs___has_windows_systemroot (guestfs_h *g)
+static int is_systemroot (guestfs_h *const g, const char *const systemroot)
 {
-  size_t i;
   char path[256];
 
-  for (i = 0; i < sizeof systemroots / sizeof systemroots[0]; ++i) {
-    CLEANUP_FREE char *systemroot =
-      guestfs___case_sensitive_path_silently (g, systemroots[i]);
-    if (!systemroot)
-      continue;
+  snprintf (path, sizeof path, "%s/system32", systemroot);
+  if (!guestfs___is_dir_nocase (g, path))
+    return 0;
 
-    snprintf (path, sizeof path, "%s/system32", systemroot);
-    if (!guestfs___is_dir_nocase (g, path))
-      continue;
+  snprintf (path, sizeof path, "%s/system32/config", systemroot);
+  if (!guestfs___is_dir_nocase (g, path))
+    return 0;
 
-    snprintf (path, sizeof path, "%s/system32/config", systemroot);
-    if (!guestfs___is_dir_nocase (g, path))
-      continue;
+  snprintf (path, sizeof path, "%s/system32/cmd.exe", systemroot);
+  if (!guestfs___is_file_nocase (g, path))
+    return 0;
+
+  return 1;
+}
+
+char *
+guestfs___get_windows_systemroot (guestfs_h *g)
+{
+  /* Check a predefined list of common windows system root locations */
+  static const char *systemroots[] =
+    { "/windows", "/winnt", "/win32", "/win", NULL };
 
-    snprintf (path, sizeof path, "%s/system32/cmd.exe", systemroot);
-    if (!guestfs___is_file_nocase (g, path))
+  for (size_t i = 0; i < sizeof systemroots / sizeof systemroots[0]; ++i) {
+    char *systemroot =
+      guestfs___case_sensitive_path_silently (g, systemroots[i]);
+    if (!systemroot)
       continue;
 
-    return (int)i;
+    if (is_systemroot (g, systemroot)) {
+      debug (g, "windows %%SYSTEMROOT%% = %s", systemroot);
+
+      return systemroot;
+    } else {
+      free (systemroot);
+    }
   }
 
-  return -1; /* not found */
+  return NULL; /* not found */
 }
 
 int
-guestfs___check_windows_root (guestfs_h *g, struct inspect_fs *fs)
+guestfs___check_windows_root (guestfs_h *g, struct inspect_fs *fs,
+                              char *const systemroot)
 {
-  int i;
-  char *systemroot;
-
   fs->type = OS_TYPE_WINDOWS;
   fs->distro = OS_DISTRO_WINDOWS;
 
-  i = guestfs___has_windows_systemroot (g);
-  if (i == -1) {
-    error (g, "check_windows_root: has_windows_systemroot unexpectedly returned -1");
-    return -1;
-  }
-
-  systemroot = guestfs_case_sensitive_path (g, systemroots[i]);
-  if (!systemroot)
-    return -1;
-
-  debug (g, "windows %%SYSTEMROOT%% = %s", systemroot);
-
   /* Freed by guestfs___free_inspect_info. */
   fs->windows_systemroot = systemroot;
 
@@ -179,7 +177,7 @@ check_windows_arch (guestfs_h *g, struct inspect_fs *fs)
   char cmd_exe[len];
   snprintf (cmd_exe, len, "%s/system32/cmd.exe", fs->windows_systemroot);
 
-  /* Should exist because of previous check above in has_windows_systemroot. */
+  /* Should exist because of previous check above in get_windows_systemroot. */
   CLEANUP_FREE char *cmd_exe_path = guestfs_case_sensitive_path (g, cmd_exe);
   if (!cmd_exe_path)
     return -1;
diff --git a/src/inspect-fs.c b/src/inspect-fs.c
index d220634..0473e92 100644
--- a/src/inspect-fs.c
+++ b/src/inspect-fs.c
@@ -175,6 +175,9 @@ check_filesystem (guestfs_h *g, const char *mountable,
                   const struct guestfs_internal_mountable *m,
                   int whole_device)
 {
+  /* Not CLEANUP_FREE, as it will be cleaned up with inspection info */
+  char *windows_systemroot = NULL;
+
   if (extend_fses (g) == -1)
     return -1;
 
@@ -274,10 +277,11 @@ check_filesystem (guestfs_h *g, const char *mountable,
            guestfs_is_dir (g, "/spool") > 0)
     ;
   /* Windows root? */
-  else if (guestfs___has_windows_systemroot (g) >= 0) {
+  else if ((windows_systemroot = guestfs___get_windows_systemroot (g)) != NULL)
+  {
     fs->is_root = 1;
     fs->format = OS_FORMAT_INSTALLED;
-    if (guestfs___check_windows_root (g, fs) == -1)
+    if (guestfs___check_windows_root (g, fs, windows_systemroot) == -1)
       return -1;
   }
   /* Windows volume with installed applications (but not root)? */
-- 
1.8.2.1