From dd2ccaaef82612f463f47f996879b8e321c19017 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 29 Sep 2020 14:22:14 +0100
Subject: [PATCH] Move _srt_file_test_in_sysroot into the library

While I'm there, add the ability to pass in a pre-opened file descriptor.

Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 pressure-vessel/runtime.c            | 64 ++++++++++++----------
 pressure-vessel/utils.c              | 71 -------------------------
 pressure-vessel/utils.h              |  4 --
 steam-runtime-tools/utils-internal.h |  6 +++
 steam-runtime-tools/utils.c          | 79 ++++++++++++++++++++++++++++
 tests/pressure-vessel/utils.c        |  4 +-
 6 files changed, 124 insertions(+), 104 deletions(-)

diff --git a/pressure-vessel/runtime.c b/pressure-vessel/runtime.c
index 1b0510a5e..b0ddafd4e 100644
--- a/pressure-vessel/runtime.c
+++ b/pressure-vessel/runtime.c
@@ -30,6 +30,7 @@
 
 #include <steam-runtime-tools/steam-runtime-tools.h>
 #include "steam-runtime-tools/resolve-in-sysroot-internal.h"
+#include "steam-runtime-tools/utils-internal.h"
 
 #include "bwrap.h"
 #include "bwrap-lock.h"
@@ -1590,8 +1591,8 @@ bind_runtime (PvRuntime *self,
   /* If we are in a Flatpak environment, we need to test if these files are
    * available in the host, and not in the current environment, because we will
    * run bwrap in the host system */
-  if (pv_file_test_in_sysroot (self->host_in_current_namespace, "/etc/machine-id",
-                               G_FILE_TEST_EXISTS))
+  if (_srt_file_test_in_sysroot (self->host_in_current_namespace, -1,
+                                 "/etc/machine-id", G_FILE_TEST_EXISTS))
     {
       flatpak_bwrap_add_args (bwrap,
                               "--ro-bind", "/etc/machine-id", "/etc/machine-id",
@@ -1602,8 +1603,9 @@ bind_runtime (PvRuntime *self,
   /* We leave this for completeness but in practice we do not expect to have
    * access to the "/var" host directory because Flatpak usually just binds
    * the host's "etc" and "usr". */
-  else if (pv_file_test_in_sysroot (self->host_in_current_namespace,
-                                    "/var/lib/dbus/machine-id", G_FILE_TEST_EXISTS))
+  else if (_srt_file_test_in_sysroot (self->host_in_current_namespace, -1,
+                                      "/var/lib/dbus/machine-id",
+                                      G_FILE_TEST_EXISTS))
     {
       flatpak_bwrap_add_args (bwrap,
                               "--ro-bind", "/var/lib/dbus/machine-id",
@@ -1613,34 +1615,34 @@ bind_runtime (PvRuntime *self,
                               NULL);
     }
 
-  if (pv_file_test_in_sysroot (self->host_in_current_namespace, "/etc/resolv.conf",
-                               G_FILE_TEST_EXISTS))
+  if (_srt_file_test_in_sysroot (self->host_in_current_namespace, -1,
+                                 "/etc/resolv.conf", G_FILE_TEST_EXISTS))
     flatpak_bwrap_add_args (bwrap,
                             "--ro-bind", "/etc/resolv.conf", "/etc/resolv.conf",
                             NULL);
 
-  if (pv_file_test_in_sysroot (self->host_in_current_namespace, "/etc/host.conf",
-                               G_FILE_TEST_EXISTS))
+  if (_srt_file_test_in_sysroot (self->host_in_current_namespace, -1,
+                                 "/etc/host.conf", G_FILE_TEST_EXISTS))
     flatpak_bwrap_add_args (bwrap,
                             "--ro-bind", "/etc/host.conf", "/etc/host.conf",
                             NULL);
 
-  if (pv_file_test_in_sysroot (self->host_in_current_namespace, "/etc/hosts",
-                               G_FILE_TEST_EXISTS))
+  if (_srt_file_test_in_sysroot (self->host_in_current_namespace, -1,
+                                 "/etc/hosts", G_FILE_TEST_EXISTS))
     flatpak_bwrap_add_args (bwrap,
                             "--ro-bind", "/etc/hosts", "/etc/hosts",
                             NULL);
 
   /* TODO: Synthesize a passwd with only the user and nobody,
    * like Flatpak does? */
-  if (pv_file_test_in_sysroot (self->host_in_current_namespace, "/etc/passwd",
-                               G_FILE_TEST_EXISTS))
+  if (_srt_file_test_in_sysroot (self->host_in_current_namespace, -1,
+                                 "/etc/passwd", G_FILE_TEST_EXISTS))
     flatpak_bwrap_add_args (bwrap,
                             "--ro-bind", "/etc/passwd", "/etc/passwd",
                             NULL);
 
-  if (pv_file_test_in_sysroot (self->host_in_current_namespace, "/etc/group",
-                               G_FILE_TEST_EXISTS))
+  if (_srt_file_test_in_sysroot (self->host_in_current_namespace, -1,
+                                 "/etc/group", G_FILE_TEST_EXISTS))
     flatpak_bwrap_add_args (bwrap,
                             "--ro-bind", "/etc/group", "/etc/group",
                             NULL);
@@ -1665,8 +1667,8 @@ bind_runtime (PvRuntime *self,
    * non-existing targets), in which case we don't want to attempt to create
    * bogus symlinks or bind mounts, as that will cause flatpak run to fail.
    */
-  if (pv_file_test_in_sysroot (self->host_in_current_namespace, "/etc/localtime",
-                               G_FILE_TEST_EXISTS))
+  if (_srt_file_test_in_sysroot (self->host_in_current_namespace, -1,
+                                 "/etc/localtime", G_FILE_TEST_EXISTS))
     {
       g_autofree char *target = NULL;
       gboolean is_reachable = FALSE;
@@ -1734,15 +1736,15 @@ pv_runtime_take_from_provider (PvRuntime *self,
 
   if (flags & TAKE_FROM_PROVIDER_FLAGS_IF_DIR)
     {
-      if (!pv_file_test_in_sysroot (self->provider_in_current_namespace,
-                                    source_in_provider, G_FILE_TEST_IS_DIR))
+      if (!_srt_file_test_in_sysroot (self->provider_in_current_namespace, -1,
+                                      source_in_provider, G_FILE_TEST_IS_DIR))
         return TRUE;
     }
 
   if (flags & TAKE_FROM_PROVIDER_FLAGS_IF_EXISTS)
     {
-      if (!pv_file_test_in_sysroot (self->provider_in_current_namespace,
-                                    source_in_provider, G_FILE_TEST_EXISTS))
+      if (!_srt_file_test_in_sysroot (self->provider_in_current_namespace, -1,
+                                      source_in_provider, G_FILE_TEST_EXISTS))
         return TRUE;
     }
 
@@ -2192,8 +2194,8 @@ pv_runtime_search_in_path_and_bin (PvRuntime *self,
       g_autofree gchar *test_path = g_build_filename (common_bin_dirs[i],
                                                       program_name,
                                                       NULL);
-      if (pv_file_test_in_sysroot (self->host_in_current_namespace,
-                                   test_path, G_FILE_TEST_IS_EXECUTABLE))
+      if (_srt_file_test_in_sysroot (self->host_in_current_namespace, -1,
+                                     test_path, G_FILE_TEST_IS_EXECUTABLE))
         return g_steal_pointer (&test_path);
     }
 
@@ -2608,8 +2610,10 @@ pv_runtime_use_provider_graphics_stack (PvRuntime *self,
 
                   gconv_dir_in_provider = g_build_filename ("/usr", dir, "gconv", NULL);
 
-                  if (pv_file_test_in_sysroot (self->provider_in_current_namespace,
-                                               gconv_dir_in_provider, G_FILE_TEST_IS_DIR))
+                  if (_srt_file_test_in_sysroot (self->provider_in_current_namespace,
+                                                 -1,
+                                                 gconv_dir_in_provider,
+                                                 G_FILE_TEST_IS_DIR))
                     {
                       g_hash_table_add (gconv_in_provider, g_steal_pointer (&gconv_dir_in_provider));
                       found = TRUE;
@@ -2638,8 +2642,10 @@ pv_runtime_use_provider_graphics_stack (PvRuntime *self,
                       g_clear_pointer (&gconv_dir_in_provider, g_free);
                       gconv_dir_in_provider = g_build_filename ("/usr", dir, "gconv", NULL);
 
-                      if (pv_file_test_in_sysroot (self->provider_in_current_namespace,
-                                                   gconv_dir_in_provider, G_FILE_TEST_IS_DIR))
+                      if (_srt_file_test_in_sysroot (self->provider_in_current_namespace,
+                                                     -1,
+                                                     gconv_dir_in_provider,
+                                                     G_FILE_TEST_IS_DIR))
                         {
                           g_hash_table_add (gconv_in_provider, g_steal_pointer (&gconv_dir_in_provider));
                           found = TRUE;
@@ -2696,8 +2702,10 @@ pv_runtime_use_provider_graphics_stack (PvRuntime *self,
 
                   libdrm_dir_in_provider = g_build_filename (dir, "share", "libdrm", NULL);
 
-                  if (pv_file_test_in_sysroot (self->provider_in_current_namespace,
-                                               libdrm_dir_in_provider, G_FILE_TEST_IS_DIR))
+                  if (_srt_file_test_in_sysroot (self->provider_in_current_namespace,
+                                                 -1,
+                                                 libdrm_dir_in_provider,
+                                                 G_FILE_TEST_IS_DIR))
                     {
                       g_hash_table_add (libdrm_data_in_provider,
                                         g_steal_pointer (&libdrm_dir_in_provider));
diff --git a/pressure-vessel/utils.c b/pressure-vessel/utils.c
index 8300b27da..5c4add5b7 100644
--- a/pressure-vessel/utils.c
+++ b/pressure-vessel/utils.c
@@ -919,74 +919,3 @@ pv_current_namespace_path_to_host_path (const gchar *current_env_path)
 
   return path_on_host;
 }
-
-/**
- * pv_file_test_in_sysroot:
- * @sysroot: (type filename): A path used as the root
- * @filename: (type filename): A path below the root directory, either
- *  absolute or relative (to the root)
- * @test: The test to perform on the resolved file in @sysroot.
- *  G_FILE_TEST_IS_SYMLINK is not a valid GFileTest value because the
- *  path is resolved following symlinks too.
- *
- * Returns: %TRUE if the @filename resolved in @sysroot passes the @test.
- */
-gboolean
-pv_file_test_in_sysroot (const char *sysroot,
-                         const char *filename,
-                         GFileTest test)
-{
-  glnx_autofd int file_fd = -1;
-  glnx_autofd int sysroot_fd = -1;
-  struct stat stat_buf;
-  g_autofree gchar *file_realpath_in_sysroot = NULL;
-  g_autoptr(GError) error = NULL;
-
-  g_return_val_if_fail (sysroot != NULL, FALSE);
-  g_return_val_if_fail (filename != NULL, FALSE);
-  /* We reject G_FILE_TEST_IS_SYMLINK because the provided filename is resolved
-   * in sysroot, following the eventual symlinks too. So it is not possible for
-   * the resolved filename to be a symlink */
-  g_return_val_if_fail ((test & (G_FILE_TEST_EXISTS | G_FILE_TEST_IS_EXECUTABLE
-                                 | G_FILE_TEST_IS_REGULAR
-                                 | G_FILE_TEST_IS_DIR)) == test, FALSE);
-
-  if (!glnx_opendirat (-1, sysroot, FALSE, &sysroot_fd, &error))
-    {
-      g_debug ("An error occurred trying to open %s: %s", sysroot,
-               error->message);
-      return FALSE;
-    }
-
-  file_fd = _srt_resolve_in_sysroot (sysroot_fd,
-                                     filename, SRT_RESOLVE_FLAGS_NONE,
-                                     &file_realpath_in_sysroot, &error);
-
-  if (file_fd < 0)
-    {
-      g_debug ("An error occurred trying to resolve %s in sysroot: %s",
-               filename, error->message);
-      return FALSE;
-    }
-
-  if (fstat (file_fd, &stat_buf) != 0)
-    {
-      g_debug ("fstat %s/%s: %s",
-               sysroot, file_realpath_in_sysroot, g_strerror (errno));
-      return FALSE;
-    }
-
-  if (test & G_FILE_TEST_EXISTS)
-    return TRUE;
-
-  if ((test & G_FILE_TEST_IS_EXECUTABLE) && (stat_buf.st_mode & 0111))
-    return TRUE;
-
-  if ((test & G_FILE_TEST_IS_REGULAR) && S_ISREG (stat_buf.st_mode))
-    return TRUE;
-
-  if ((test & G_FILE_TEST_IS_DIR) && S_ISDIR (stat_buf.st_mode))
-    return TRUE;
-
-  return FALSE;
-}
diff --git a/pressure-vessel/utils.h b/pressure-vessel/utils.h
index 380910472..a30011e09 100644
--- a/pressure-vessel/utils.h
+++ b/pressure-vessel/utils.h
@@ -71,7 +71,3 @@ gboolean pv_terminate_all_child_processes (GTimeSpan wait_period,
                                            GError **error);
 
 gchar *pv_current_namespace_path_to_host_path (const gchar *current_env_path);
-
-gboolean pv_file_test_in_sysroot (const char *sysroot,
-                                  const char *filename,
-                                  GFileTest test);
diff --git a/steam-runtime-tools/utils-internal.h b/steam-runtime-tools/utils-internal.h
index ca7f38f49..dc5d7c92c 100644
--- a/steam-runtime-tools/utils-internal.h
+++ b/steam-runtime-tools/utils-internal.h
@@ -94,3 +94,9 @@ gboolean _srt_file_get_contents_in_sysroot (int sysroot_fd,
                                             gchar **contents,
                                             gsize *len,
                                             GError **error);
+
+G_GNUC_INTERNAL
+gboolean _srt_file_test_in_sysroot (const char *sysroot,
+                                    int sysroot_fd,
+                                    const char *filename,
+                                    GFileTest test);
diff --git a/steam-runtime-tools/utils.c b/steam-runtime-tools/utils.c
index 55d2917ed..223a06565 100644
--- a/steam-runtime-tools/utils.c
+++ b/steam-runtime-tools/utils.c
@@ -917,3 +917,82 @@ _srt_file_get_contents_in_sysroot (int sysroot_fd,
 
   return TRUE;
 }
+
+/*
+ * _srt_file_test_in_sysroot:
+ * @sysroot: (type filename): A path used as the root
+ * @sysroot_fd: A file descriptor opened on @sysroot, or negative to
+ *  reopen it
+ * @filename: (type filename): A path below the root directory, either
+ *  absolute or relative (to the root)
+ * @test: The test to perform on the resolved file in @sysroot.
+ *  G_FILE_TEST_IS_SYMLINK is not a valid GFileTest value because the
+ *  path is resolved following symlinks too.
+ *
+ * Returns: %TRUE if the @filename resolved in @sysroot passes the @test.
+ */
+gboolean
+_srt_file_test_in_sysroot (const char *sysroot,
+                           int sysroot_fd,
+                           const char *filename,
+                           GFileTest test)
+{
+  glnx_autofd int file_fd = -1;
+  glnx_autofd int local_sysroot_fd = -1;
+  struct stat stat_buf;
+  g_autofree gchar *file_realpath_in_sysroot = NULL;
+  g_autoptr(GError) error = NULL;
+
+  g_return_val_if_fail (sysroot != NULL, FALSE);
+  g_return_val_if_fail (filename != NULL, FALSE);
+  /* We reject G_FILE_TEST_IS_SYMLINK because the provided filename is resolved
+   * in sysroot, following the eventual symlinks too. So it is not possible for
+   * the resolved filename to be a symlink */
+  g_return_val_if_fail ((test & (G_FILE_TEST_EXISTS | G_FILE_TEST_IS_EXECUTABLE
+                                 | G_FILE_TEST_IS_REGULAR
+                                 | G_FILE_TEST_IS_DIR)) == test, FALSE);
+
+  if (sysroot_fd < 0)
+    {
+      if (!glnx_opendirat (-1, sysroot, FALSE, &local_sysroot_fd, &error))
+        {
+          g_debug ("An error occurred trying to open %s: %s", sysroot,
+                   error->message);
+          return FALSE;
+        }
+
+      sysroot_fd = local_sysroot_fd;
+    }
+
+  file_fd = _srt_resolve_in_sysroot (sysroot_fd,
+                                     filename, SRT_RESOLVE_FLAGS_NONE,
+                                     &file_realpath_in_sysroot, &error);
+
+  if (file_fd < 0)
+    {
+      g_debug ("An error occurred trying to resolve %s in sysroot: %s",
+               filename, error->message);
+      return FALSE;
+    }
+
+  if (fstat (file_fd, &stat_buf) != 0)
+    {
+      g_debug ("fstat %s/%s: %s",
+               sysroot, file_realpath_in_sysroot, g_strerror (errno));
+      return FALSE;
+    }
+
+  if (test & G_FILE_TEST_EXISTS)
+    return TRUE;
+
+  if ((test & G_FILE_TEST_IS_EXECUTABLE) && (stat_buf.st_mode & 0111))
+    return TRUE;
+
+  if ((test & G_FILE_TEST_IS_REGULAR) && S_ISREG (stat_buf.st_mode))
+    return TRUE;
+
+  if ((test & G_FILE_TEST_IS_DIR) && S_ISDIR (stat_buf.st_mode))
+    return TRUE;
+
+  return FALSE;
+}
diff --git a/tests/pressure-vessel/utils.c b/tests/pressure-vessel/utils.c
index 5b396fa89..f400bc7de 100644
--- a/tests/pressure-vessel/utils.c
+++ b/tests/pressure-vessel/utils.c
@@ -30,6 +30,7 @@
 #include <glib/gstdio.h>
 
 #include "steam-runtime-tools/glib-backports-internal.h"
+#include "steam-runtime-tools/utils-internal.h"
 #include "libglnx/libglnx.h"
 
 #include "tests/test-utils.h"
@@ -374,7 +375,8 @@ test_file_in_sysroot (Fixture *f,
     {
       const InSysrootTest *it = &tests[i];
 
-      g_assert_cmpint (pv_file_test_in_sysroot (tmpdir.path, it->path, it->test),
+      g_assert_cmpint (_srt_file_test_in_sysroot (tmpdir.path, -1,
+                                                  it->path, it->test),
                        ==, it->expected_result);
     }
 }
-- 
GitLab