From 78f69eb69bb585ae0632c3724e5950eb0319dd50 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Thu, 5 Mar 2020 12:00:21 +0000
Subject: [PATCH] Combine common code for dealing with graphics modules

Resolves: #8
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 steam-runtime-tools/graphics-internal.h |  23 +--
 steam-runtime-tools/graphics.c          |  94 ++-------
 steam-runtime-tools/system-info.c       | 254 +++++++++---------------
 tests/graphics.c                        |   2 +
 4 files changed, 122 insertions(+), 251 deletions(-)

diff --git a/steam-runtime-tools/graphics-internal.h b/steam-runtime-tools/graphics-internal.h
index d6723ac63..824f1d1dd 100644
--- a/steam-runtime-tools/graphics-internal.h
+++ b/steam-runtime-tools/graphics-internal.h
@@ -147,12 +147,14 @@ _srt_graphics_rendering_interface_string (SrtRenderingInterface rendering_interf
  * @SRT_GRAPHICS_DRI_MODULE: Mesa DRI driver module
  * @SRT_GRAPHICS_VAAPI_MODULE: VA-API driver module
  * @SRT_GRAPHICS_VDPAU_MODULE: VDPAU driver module
+ * @NUM_SRT_GRAPHICS_MODULES: 1 more than the last valid enum value
  */
 typedef enum
 {
   SRT_GRAPHICS_DRI_MODULE = 0,
   SRT_GRAPHICS_VAAPI_MODULE,
   SRT_GRAPHICS_VDPAU_MODULE,
+  NUM_SRT_GRAPHICS_MODULES  /* Always add new values before this one */
 } SrtGraphicsModule;
 
 G_GNUC_INTERNAL
@@ -165,19 +167,8 @@ GList *_srt_load_vulkan_icds (const char *sysroot,
                               const char * const *multiarch_tuples);
 
 G_GNUC_INTERNAL
-GList *_srt_list_va_api_drivers (const char *sysroot,
-                                 gchar **envp,
-                                 const char *helpers_path,
-                                 const char *multiarch_tuple);
-
-G_GNUC_INTERNAL
-GList *_srt_list_dri_drivers (const char *sysroot,
-                              gchar **envp,
-                              const char *helpers_path,
-                              const char *multiarch_tuple);
-
-G_GNUC_INTERNAL
-GList *_srt_list_vdpau_drivers (const char *sysroot,
-                                gchar **envp,
-                                const char *helpers_path,
-                                const char *multiarch_tuple);
+GList *_srt_list_graphics_modules (const gchar *sysroot,
+                                   gchar **envp,
+                                   const char *helpers_path,
+                                   const char *multiarch_tuple,
+                                   SrtGraphicsModule which);
diff --git a/steam-runtime-tools/graphics.c b/steam-runtime-tools/graphics.c
index 3e2b1679f..9ed18455a 100644
--- a/steam-runtime-tools/graphics.c
+++ b/steam-runtime-tools/graphics.c
@@ -2513,6 +2513,7 @@ _srt_get_modules_from_path (gchar **envp,
         module_suffix[2] = NULL;
         break;
 
+      case NUM_SRT_GRAPHICS_MODULES:
       default:
         g_return_if_reached ();
     }
@@ -2564,6 +2565,7 @@ _srt_get_modules_from_path (gchar **envp,
                 g_free (this_driver_link);
                 break;
 
+              case NUM_SRT_GRAPHICS_MODULES:
               default:
                 g_return_if_reached ();
             }
@@ -2632,6 +2634,7 @@ _srt_get_modules_full (const char *sysroot,
         env_override = "VDPAU_DRIVER_PATH";
         break;
 
+      case NUM_SRT_GRAPHICS_MODULES:
       default:
         g_return_if_reached ();
     }
@@ -2859,37 +2862,38 @@ _srt_get_modules_full (const char *sysroot,
 }
 
 /**
- * _srt_list_dri_drivers:
+ * _srt_list_graphics_modules:
  * @sysroot: (nullable): Look in this directory instead of the real root
  * @envp: (array zero-terminated=1): Behave as though `environ` was this array
  * @helpers_path: (nullable): An optional path to find "inspect-library" helper, PATH is used if %NULL
  * @multiarch_tuple: (not nullable) (type filename): A Debian-style multiarch tuple
  *  such as %SRT_ABI_X86_64
+ * @which: Graphics modules to look for
  *
- * Implementation of srt_system_info_list_dri_drivers().
+ * Implementation of srt_system_info_list_dri_drivers() etc.
  *
  * The returned list will have the most-preferred directories first and the
  * least-preferred directories last. Within a directory, the drivers will be in
  * lexicographic order, for example `nouveau_dri.so`, `r200_dri.so`, `r600_dri.so`
  * in that order.
  *
- * Returns: (transfer full) (element-type SrtDriDriver) (nullable): A list of
- *  opaque #SrtDriDriver objects, or %NULL if nothing was found. Free with
+ * Returns: (transfer full) (element-type GObject) (nullable): A list of
+ *  opaque #SrtDriDriver, etc. objects, or %NULL if nothing was found. Free with
  *  `g_list_free_full(list, g_object_unref)`.
  */
 GList *
-_srt_list_dri_drivers (const char *sysroot,
-                       gchar **envp,
-                       const char *helpers_path,
-                       const char *multiarch_tuple)
+_srt_list_graphics_modules (const char *sysroot,
+                            gchar **envp,
+                            const char *helpers_path,
+                            const char *multiarch_tuple,
+                            SrtGraphicsModule which)
 {
   GList *drivers = NULL;
 
   g_return_val_if_fail (multiarch_tuple != NULL, NULL);
 
   _srt_get_modules_full (sysroot, envp, helpers_path, multiarch_tuple,
-                         SRT_GRAPHICS_DRI_MODULE, &drivers);
-
+                         which, &drivers);
   return g_list_reverse (drivers);
 }
 
@@ -3063,41 +3067,6 @@ srt_va_api_driver_is_extra (SrtVaApiDriver *self)
   return self->is_extra;
 }
 
-/*
- * _srt_list_va_api_drivers:
- * @sysroot: (nullable): Look in this directory instead of the real root
- * @envp: (array zero-terminated=1): Behave as though `environ` was this array
- * @helpers_path: (nullable): An optional path to find "inspect-library" helper, PATH is used if %NULL
- * @multiarch_tuple: (not nullable) (type filename): A Debian-style multiarch tuple
- *  such as %SRT_ABI_X86_64
- *
- * Implementation of srt_system_info_list_va_api_drivers().
- *
- * The returned list will have the most-preferred directories first and the
- * least-preferred directories last. Within a directory, the drivers will be in
- * lexicographic order, for example `nouveau_drv_video.so`, `r600_drv_video.so`,
- * `radeonsi_drv_video.so` in that order.
- *
- * Returns: (transfer full) (element-type SrtVaApiDriver) (nullable): A list of
- *  opaque #SrtVaApiDriver objects, or %NULL if nothing was found. Free with
- *  `g_list_free_full(list, g_object_unref)`.
- */
-GList *
-_srt_list_va_api_drivers (const char *sysroot,
-                          gchar **envp,
-                          const char *helpers_path,
-                          const char *multiarch_tuple)
-{
-  GList *drivers = NULL;
-
-  g_return_val_if_fail (multiarch_tuple != NULL, NULL);
-
-  _srt_get_modules_full (sysroot, envp, helpers_path, multiarch_tuple,
-                         SRT_GRAPHICS_VAAPI_MODULE, &drivers);
-
-  return g_list_reverse (drivers);
-}
-
 /**
  * SrtVdpauDriver:
  *
@@ -3306,41 +3275,6 @@ srt_vdpau_driver_is_extra (SrtVdpauDriver *self)
   return self->is_extra;
 }
 
-/*
- * _srt_list_vdpau_drivers:
- * @sysroot: (nullable): Look in this directory instead of the real root
- * @envp: (array zero-terminated=1): Behave as though `environ` was this array
- * @helpers_path: (nullable): An optional path to find "inspect-library" helper, PATH is used if %NULL
- * @multiarch_tuple: (not nullable) (type filename): A Debian-style multiarch tuple
- *  such as %SRT_ABI_X86_64
- *
- * Implementation of srt_system_info_list_vdpau_drivers().
- *
- * The returned list will have the most-preferred directories first and the
- * least-preferred directories last. Within a directory, the drivers will be in
- * lexicographic order, for example `libvdpau_nouveau.so`, `libvdpau_r600.so`,
- * `libvdpau_radeonsi.so` in that order.
- *
- * Returns: (transfer full) (element-type SrtVdpauDriver) (nullable): A list of
- *  opaque #SrtVdpauDriver objects, or %NULL if nothing was found. Free with
- *  `g_list_free_full(list, g_object_unref)`.
- */
-GList *
-_srt_list_vdpau_drivers (const char *sysroot,
-                         gchar **envp,
-                         const char *helpers_path,
-                         const char *multiarch_tuple)
-{
-  GList *drivers = NULL;
-
-  g_return_val_if_fail (multiarch_tuple != NULL, NULL);
-
-  _srt_get_modules_full (sysroot, envp, helpers_path, multiarch_tuple,
-                         SRT_GRAPHICS_VDPAU_MODULE, &drivers);
-
-  return g_list_reverse (drivers);
-}
-
 /**
  * SrtVulkanIcd:
  *
diff --git a/steam-runtime-tools/system-info.c b/steam-runtime-tools/system-info.c
index c336542b1..63dc2c409 100644
--- a/steam-runtime-tools/system-info.c
+++ b/steam-runtime-tools/system-info.c
@@ -214,6 +214,12 @@ maybe_locale_free (gpointer p)
   g_slice_free (MaybeLocale, self);
 }
 
+typedef struct
+{
+  GList *modules;
+  gboolean available;
+} ModuleList;
+
 typedef struct
 {
   GQuark multiarch_tuple;
@@ -226,14 +232,7 @@ typedef struct
   SrtGraphicsIssues cached_combined_graphics_issues;
   gboolean graphics_cache_available;
 
-  GList *cached_dri_list;
-  gboolean dri_cache_available;
-
-  GList *cached_va_api_list;
-  gboolean va_api_cache_available;
-
-  GList *cached_vdpau_list;
-  gboolean vdpau_cache_available;
+  ModuleList graphics_modules[NUM_SRT_GRAPHICS_MODULES];
 } Abi;
 
 static Abi *
@@ -262,12 +261,12 @@ ensure_abi (SrtSystemInfo *self,
   abi->libraries_cache_available = FALSE;
   abi->cached_graphics_results = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_object_unref);
   abi->cached_combined_graphics_issues = SRT_GRAPHICS_ISSUES_NONE;
-  abi->cached_dri_list = NULL;
-  abi->dri_cache_available = FALSE;
-  abi->cached_va_api_list = NULL;
-  abi->va_api_cache_available = FALSE;
-  abi->cached_vdpau_list = NULL;
-  abi->vdpau_cache_available = FALSE;
+
+  for (i = 0; i < G_N_ELEMENTS (abi->graphics_modules); i++)
+    {
+      abi->graphics_modules[i].modules = NULL;
+      abi->graphics_modules[i].available = FALSE;
+    }
 
   /* transfer ownership to self->abis */
   g_ptr_array_add (self->abis, abi);
@@ -278,15 +277,16 @@ static void
 abi_free (gpointer self)
 {
   Abi *abi = self;
+  gsize i;
+
   if (abi->cached_results != NULL)
     g_hash_table_unref (abi->cached_results);
 
   if (abi->cached_graphics_results != NULL)
     g_hash_table_unref (abi->cached_graphics_results);
 
-  g_list_free_full (abi->cached_dri_list, g_object_unref);
-  g_list_free_full (abi->cached_va_api_list, g_object_unref);
-  g_list_free_full (abi->cached_vdpau_list, g_object_unref);
+  for (i = 0; i < G_N_ELEMENTS (abi->graphics_modules); i++)
+    g_list_free_full (abi->graphics_modules[i].modules, g_object_unref);
 
   g_slice_free (Abi, self);
 }
@@ -1393,56 +1393,23 @@ forget_graphics_results (SrtSystemInfo *self)
 }
 
 /*
- * Forget any cached information about mesa DRI drivers.
- */
-static void
-forget_dris (SrtSystemInfo *self)
-{
-  gsize i;
-
-  for (i = 0; i < self->abis->len; i++)
-    {
-      Abi *abi = g_ptr_array_index (self->abis, i);
-
-      g_list_free_full (abi->cached_dri_list, g_object_unref);
-      abi->cached_dri_list = NULL;
-      abi->dri_cache_available = FALSE;
-    }
-}
-
-/*
- * Forget any cached information about VA-API drivers.
+ * Forget any cached information about graphics modules.
  */
 static void
-forget_va_apis (SrtSystemInfo *self)
+forget_graphics_modules (SrtSystemInfo *self)
 {
-  gsize i;
+  gsize i, j;
 
   for (i = 0; i < self->abis->len; i++)
     {
       Abi *abi = g_ptr_array_index (self->abis, i);
 
-      g_list_free_full (abi->cached_va_api_list, g_object_unref);
-      abi->cached_va_api_list = NULL;
-      abi->va_api_cache_available = FALSE;
-    }
-}
-
-/*
- * Forget any cached information about VDPAU drivers.
- */
-static void
-forget_vdpau (SrtSystemInfo *self)
-{
-  gsize i;
-
-  for (i = 0; i < self->abis->len; i++)
-    {
-      Abi *abi = g_ptr_array_index (self->abis, i);
-
-      g_list_free_full (abi->cached_vdpau_list, g_object_unref);
-      abi->cached_vdpau_list = NULL;
-      abi->vdpau_cache_available = FALSE;
+      for (j = 0; j < G_N_ELEMENTS (abi->graphics_modules); j++)
+        {
+          g_list_free_full (g_steal_pointer (&abi->graphics_modules[j].modules),
+                            g_object_unref);
+          abi->graphics_modules[j].available = FALSE;
+        }
     }
 }
 
@@ -1596,9 +1563,7 @@ srt_system_info_set_environ (SrtSystemInfo *self,
 {
   g_return_if_fail (SRT_IS_SYSTEM_INFO (self));
 
-  forget_dris (self);
-  forget_va_apis (self);
-  forget_vdpau (self);
+  forget_graphics_modules (self);
   forget_libraries (self);
   forget_graphics_results (self);
   forget_locales (self);
@@ -1627,9 +1592,7 @@ srt_system_info_set_sysroot (SrtSystemInfo *self,
 {
   g_return_if_fail (SRT_IS_SYSTEM_INFO (self));
 
-  forget_dris (self);
-  forget_va_apis (self);
-  forget_vdpau (self);
+  forget_graphics_modules (self);
   forget_libraries (self);
   forget_graphics_results (self);
   forget_locales (self);
@@ -2201,9 +2164,7 @@ srt_system_info_set_helpers_path (SrtSystemInfo *self,
 {
   g_return_if_fail (SRT_IS_SYSTEM_INFO (self));
 
-  forget_dris (self);
-  forget_va_apis (self);
-  forget_vdpau (self);
+  forget_graphics_modules (self);
   forget_libraries (self);
   forget_graphics_results (self);
   forget_locales (self);
@@ -2538,6 +2499,70 @@ srt_system_info_list_vulkan_icds (SrtSystemInfo *self,
   return g_list_reverse (ret);
 }
 
+/* Maybe they should implement a common GInterface or have a common
+ * base class or something, but for now we do this the easy way */
+static gboolean
+graphics_module_is_extra (SrtGraphicsModule which,
+                          gpointer object)
+{
+  switch (which)
+    {
+      case SRT_GRAPHICS_DRI_MODULE:
+        return srt_dri_driver_is_extra (object);
+
+      case SRT_GRAPHICS_VAAPI_MODULE:
+        return srt_va_api_driver_is_extra (object);
+
+      case SRT_GRAPHICS_VDPAU_MODULE:
+        return srt_vdpau_driver_is_extra (object);
+
+      case NUM_SRT_GRAPHICS_MODULES:
+      default:
+        g_return_val_if_reached (FALSE);
+    }
+}
+
+static GList *
+_srt_system_info_list_graphics_modules (SrtSystemInfo *self,
+                                        const char *multiarch_tuple,
+                                        SrtDriverFlags flags,
+                                        SrtGraphicsModule which)
+{
+  Abi *abi = NULL;
+  GList *ret = NULL;
+  const GList *iter;
+
+  g_return_val_if_fail (SRT_IS_SYSTEM_INFO (self), NULL);
+  g_return_val_if_fail (multiarch_tuple != NULL, NULL);
+  g_return_val_if_fail ((int) which >= 0, NULL);
+  g_return_val_if_fail ((int) which < NUM_SRT_GRAPHICS_MODULES, NULL);
+
+  abi = ensure_abi (self, multiarch_tuple);
+
+  if (!abi->graphics_modules[which].available)
+    {
+      abi->graphics_modules[which].modules = _srt_list_graphics_modules (self->sysroot,
+                                                                         self->env,
+                                                                         self->helpers_path,
+                                                                         multiarch_tuple,
+                                                                         which);
+      abi->graphics_modules[which].available = TRUE;
+    }
+
+  for (iter = abi->graphics_modules[which].modules;
+       iter != NULL;
+       iter = iter->next)
+    {
+      if ((flags & SRT_DRIVER_FLAGS_INCLUDE_ALL) == 0 &&
+          graphics_module_is_extra (which, iter->data))
+        continue;
+
+      ret = g_list_prepend (ret, g_object_ref (iter->data));
+    }
+
+  return g_list_reverse (ret);
+}
+
 /**
  * srt_system_info_list_dri_drivers:
  * @self: The #SrtSystemInfo object
@@ -2566,34 +2591,8 @@ srt_system_info_list_dri_drivers (SrtSystemInfo *self,
                                   const char *multiarch_tuple,
                                   SrtDriverFlags flags)
 {
-  Abi *abi = NULL;
-  GList *ret = NULL;
-  const GList *iter;
-
-  g_return_val_if_fail (SRT_IS_SYSTEM_INFO (self), NULL);
-  g_return_val_if_fail (multiarch_tuple != NULL, NULL);
-
-  abi = ensure_abi (self, multiarch_tuple);
-
-  if (!abi->dri_cache_available)
-    {
-      abi->cached_dri_list = _srt_list_dri_drivers (self->sysroot,
-                                                    self->env,
-                                                    self->helpers_path,
-                                                    multiarch_tuple);
-      abi->dri_cache_available = TRUE;
-    }
-
-  for (iter = abi->cached_dri_list; iter != NULL; iter = iter->next)
-    {
-      if ((flags & SRT_DRIVER_FLAGS_INCLUDE_ALL) == 0 &&
-          srt_dri_driver_is_extra (iter->data))
-        continue;
-
-      ret = g_list_prepend (ret, g_object_ref (iter->data));
-    }
-
-  return g_list_reverse (ret);
+  return _srt_system_info_list_graphics_modules (self, multiarch_tuple, flags,
+                                                 SRT_GRAPHICS_DRI_MODULE);
 }
 
 /**
@@ -2624,33 +2623,8 @@ srt_system_info_list_va_api_drivers (SrtSystemInfo *self,
                                      const char *multiarch_tuple,
                                      SrtDriverFlags flags)
 {
-  Abi *abi = NULL;
-  GList *ret = NULL;
-  const GList *iter;
-
-  g_return_val_if_fail (SRT_IS_SYSTEM_INFO (self), NULL);
-  g_return_val_if_fail (multiarch_tuple != NULL, NULL);
-
-  abi = ensure_abi (self, multiarch_tuple);
-
-  if (!abi->va_api_cache_available)
-    {
-      abi->cached_va_api_list = _srt_list_va_api_drivers (self->sysroot,
-                                                          self->env,
-                                                          self->helpers_path,
-                                                          multiarch_tuple);
-      abi->va_api_cache_available = TRUE;
-    }
-
-  for (iter = abi->cached_va_api_list; iter != NULL; iter = iter->next)
-    {
-      if ((flags & SRT_DRIVER_FLAGS_INCLUDE_ALL) == 0 &&
-          srt_va_api_driver_is_extra (iter->data))
-        continue;
-      ret = g_list_prepend (ret, g_object_ref (iter->data));
-    }
-
-  return g_list_reverse (ret);
+  return _srt_system_info_list_graphics_modules (self, multiarch_tuple, flags,
+                                                 SRT_GRAPHICS_VAAPI_MODULE);
 }
 
 /**
@@ -2681,38 +2655,8 @@ srt_system_info_list_vdpau_drivers (SrtSystemInfo *self,
                                     const char *multiarch_tuple,
                                     SrtDriverFlags flags)
 {
-  Abi *abi = NULL;
-  GList *ret = NULL;
-  const GList *iter;
-
-  g_return_val_if_fail (SRT_IS_SYSTEM_INFO (self), NULL);
-  g_return_val_if_fail (multiarch_tuple != NULL, NULL);
-
-  abi = ensure_abi (self, multiarch_tuple);
-
-  if (!abi->vdpau_cache_available)
-    {
-      abi->cached_vdpau_list = _srt_list_vdpau_drivers (self->sysroot,
-                                                        self->env,
-                                                        self->helpers_path,
-                                                        multiarch_tuple);
-      abi->vdpau_cache_available = TRUE;
-    }
-
-  for (iter = abi->cached_vdpau_list; iter != NULL; iter = iter->next)
-    {
-      if ((flags & SRT_DRIVER_FLAGS_INCLUDE_ALL) == 0 &&
-          srt_vdpau_driver_is_extra (iter->data))
-        continue;
-      ret = g_list_prepend (ret, g_object_ref (iter->data));
-    }
-
-  for (const GList *iteri = ret; iteri != NULL; iteri = iteri->next)
-    {
-      g_debug ("LIST VDPAU %s", srt_vdpau_driver_get_library_path (iteri->data));
-    }
-
-  return g_list_reverse (ret);
+  return _srt_system_info_list_graphics_modules (self, multiarch_tuple, flags,
+                                                 SRT_GRAPHICS_VDPAU_MODULE);
 }
 
 static void
diff --git a/tests/graphics.c b/tests/graphics.c
index 1e3f0b487..585734441 100644
--- a/tests/graphics.c
+++ b/tests/graphics.c
@@ -1722,6 +1722,7 @@ check_list_extra (const GList *list,
             g_assert_cmpint (is_extra, ==, srt_vdpau_driver_is_extra (iter->data));
             break;
 
+          case NUM_SRT_GRAPHICS_MODULES:
           default:
             g_return_if_reached ();
         }
@@ -1745,6 +1746,7 @@ check_list_links (const GList *list,
 
           case SRT_GRAPHICS_VAAPI_MODULE:
           case SRT_GRAPHICS_DRI_MODULE:
+          case NUM_SRT_GRAPHICS_MODULES:
           default:
             g_return_if_reached ();
         }
-- 
GitLab