From 6be80b41eb737977c270752f579757b4aa26af95 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Mon, 4 Nov 2019 18:25:42 +0000 Subject: [PATCH] _srt_get_helpers_path: Raise a GError This function initially always returned a non-NULL path even in error conditions, but having structured error handling in all callers lets us make it "failable". Signed-off-by: Simon McVittie <smcv@collabora.com> --- steam-runtime-tools/architecture.c | 8 +++++- steam-runtime-tools/graphics.c | 42 +++++++++++++++++++--------- steam-runtime-tools/library.c | 17 ++++++++--- steam-runtime-tools/locale.c | 9 ++++-- steam-runtime-tools/utils-internal.h | 2 +- steam-runtime-tools/utils.c | 18 +++++------- 6 files changed, 63 insertions(+), 33 deletions(-) diff --git a/steam-runtime-tools/architecture.c b/steam-runtime-tools/architecture.c index 00f2d6720..3439ab757 100644 --- a/steam-runtime-tools/architecture.c +++ b/steam-runtime-tools/architecture.c @@ -59,7 +59,13 @@ _srt_architecture_can_run (const char *helpers_path, g_return_val_if_fail (_srt_check_not_setuid (), FALSE); if (helpers_path == NULL) - helpers_path = _srt_get_helpers_path (); + helpers_path = _srt_get_helpers_path (&error); + + if (helpers_path == NULL) + { + g_debug ("%s", error->message); + goto out; + } helper = g_strdup_printf ("%s/%s-true", helpers_path, multiarch); argv[0] = helper; diff --git a/steam-runtime-tools/graphics.c b/steam-runtime-tools/graphics.c index 56ad224db..c716503ca 100644 --- a/steam-runtime-tools/graphics.c +++ b/steam-runtime-tools/graphics.c @@ -545,7 +545,8 @@ _argv_for_graphics_test (const char *helpers_path, static GPtrArray * _argv_for_check_vulkan (const char *helpers_path, SrtTestFlags test_flags, - const char *multiarch_tuple) + const char *multiarch_tuple, + GError **error) { GPtrArray *argv = g_ptr_array_new_with_free_func (g_free); @@ -568,7 +569,13 @@ _argv_for_check_vulkan (const char *helpers_path, } if (helpers_path == NULL) - helpers_path = _srt_get_helpers_path (); + helpers_path = _srt_get_helpers_path (error); + + if (helpers_path == NULL) + { + g_ptr_array_unref (argv); + return NULL; + } g_ptr_array_add (argv, g_strdup_printf ("%s/%s-check-vulkan", helpers_path, multiarch_tuple)); g_ptr_array_add (argv, NULL); @@ -698,9 +705,16 @@ _srt_check_graphics (const char *helpers_path, argv = _argv_for_check_vulkan (helpers_path, test_flags, - multiarch_tuple); + multiarch_tuple, + &error); - g_return_val_if_fail (argv != NULL, SRT_GRAPHICS_ISSUES_INTERNAL_ERROR); + if (argv == NULL) + { + issues |= SRT_GRAPHICS_ISSUES_CANNOT_DRAW; + /* Put the error message in the 'messages' */ + child_stderr2 = g_strdup (error->message); + goto out; + } /* Now run and report exit code/messages if failure */ if (!g_spawn_sync (NULL, /* working directory */ @@ -719,15 +733,6 @@ _srt_check_graphics (const char *helpers_path, goto out; } - if (child_stderr2 != NULL && child_stderr2[0] != '\0') - { - gchar *tmp = g_strconcat (child_stderr, child_stderr2, NULL); - - g_free (child_stderr); - child_stderr = tmp; - } - g_free (child_stderr2); - if (exit_status != 0) { g_debug ("... wait status %d", exit_status); @@ -743,6 +748,16 @@ _srt_check_graphics (const char *helpers_path, } out: + /* If we have stderr (or error messages) from both vulkaninfo and + * check-vulkan, combine them */ + if (child_stderr2 != NULL && child_stderr2[0] != '\0') + { + gchar *tmp = g_strconcat (child_stderr, child_stderr2, NULL); + + g_free (child_stderr); + child_stderr = tmp; + } + if (details_out != NULL) *details_out = _srt_graphics_new (multiarch_tuple, window_system, @@ -759,6 +774,7 @@ out: g_ptr_array_unref (argv); g_free (output); g_free (child_stderr); + g_free (child_stderr2); g_clear_error (&error); g_free (filtered_preload); g_strfreev (my_environ); diff --git a/steam-runtime-tools/library.c b/steam-runtime-tools/library.c index bf9ae8646..0949a6e17 100644 --- a/steam-runtime-tools/library.c +++ b/steam-runtime-tools/library.c @@ -514,8 +514,20 @@ _srt_check_library_presence (const char *helpers_path, SRT_LIBRARY_ISSUES_INTERNAL_ERROR); g_return_val_if_fail (_srt_check_not_setuid (), SRT_LIBRARY_ISSUES_INTERNAL_ERROR); + if (symbols_path == NULL) + issues |= SRT_LIBRARY_ISSUES_UNKNOWN_EXPECTATIONS; + if (helpers_path == NULL) - helpers_path = _srt_get_helpers_path (); + helpers_path = _srt_get_helpers_path (&error); + + if (helpers_path == NULL) + { + issues |= SRT_LIBRARY_ISSUES_CANNOT_LOAD; + /* Use the error message as though the child had printed it on stderr - + * either way, it's a useful diagnostic */ + child_stderr = g_strdup (error->message); + goto out; + } g_ptr_array_add (argv, g_strdup_printf ("%s/%s-inspect-library", helpers_path, multiarch)); g_debug ("Checking library %s integrity with %s", soname, (gchar *)g_ptr_array_index (argv, 0)); @@ -546,9 +558,6 @@ _srt_check_library_presence (const char *helpers_path, /* NULL terminate the array */ g_ptr_array_add (argv, NULL); - if (symbols_path == NULL) - issues |= SRT_LIBRARY_ISSUES_UNKNOWN_EXPECTATIONS; - my_environ = g_get_environ (); ld_preload = g_environ_getenv (my_environ, "LD_PRELOAD"); if (ld_preload != NULL) diff --git a/steam-runtime-tools/locale.c b/steam-runtime-tools/locale.c index e04c14a41..9fbcb0509 100644 --- a/steam-runtime-tools/locale.c +++ b/steam-runtime-tools/locale.c @@ -225,6 +225,12 @@ _srt_check_locale (const char *helpers_path, g_return_val_if_fail (requested_name != NULL, NULL); g_return_val_if_fail (_srt_check_not_setuid (), NULL); + if (helpers_path == NULL) + helpers_path = _srt_get_helpers_path (error); + + if (helpers_path == NULL) + goto out; + my_environ = g_get_environ (); ld_preload = g_environ_getenv (my_environ, "LD_PRELOAD"); if (ld_preload != NULL) @@ -236,9 +242,6 @@ _srt_check_locale (const char *helpers_path, argv = g_ptr_array_new_with_free_func (g_free); - if (helpers_path == NULL) - helpers_path = _srt_get_helpers_path (); - if (multiarch_tuple == NULL) multiarch_tuple = _SRT_MULTIARCH; diff --git a/steam-runtime-tools/utils-internal.h b/steam-runtime-tools/utils-internal.h index 8fc5b0eca..f24635136 100644 --- a/steam-runtime-tools/utils-internal.h +++ b/steam-runtime-tools/utils-internal.h @@ -28,5 +28,5 @@ #include <glib.h> -G_GNUC_INTERNAL const char *_srt_get_helpers_path (void); +G_GNUC_INTERNAL const char *_srt_get_helpers_path (GError **error); gchar *_srt_filter_gameoverlayrenderer (const gchar *input); diff --git a/steam-runtime-tools/utils.c b/steam-runtime-tools/utils.c index c8a06acdd..048e72933 100644 --- a/steam-runtime-tools/utils.c +++ b/steam-runtime-tools/utils.c @@ -39,6 +39,7 @@ #endif #include <glib-object.h> +#include <gio/gio.h> #ifdef HAVE_GETAUXVAL #define getauxval_AT_SECURE() getauxval (AT_SECURE) @@ -147,14 +148,15 @@ _srt_check_not_setuid (void) static gchar *helpers_path = NULL; G_GNUC_INTERNAL const char * -_srt_get_helpers_path (void) +_srt_get_helpers_path (GError **error) { const char *path; Dl_info ignored; struct link_map *map = NULL; gchar *dir; - g_return_val_if_fail (_srt_check_not_setuid (), "/"); + g_return_val_if_fail (_srt_check_not_setuid (), NULL); + g_return_val_if_fail (error == NULL || *error == NULL, NULL); path = helpers_path; @@ -170,8 +172,9 @@ _srt_get_helpers_path (void) RTLD_DL_LINKMAP) == 0 || map == NULL) { - g_warning ("Unable to locate shared library containing " - "_srt_get_helpers_path()"); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Unable to locate shared library containing " + "_srt_get_helpers_path()"); goto out; } @@ -203,13 +206,6 @@ _srt_get_helpers_path (void) g_free (dir); out: - /* We have to return *something* non-NULL */ - if (path == NULL) - { - g_warning ("Unable to determine path to helpers"); - path = "/"; - } - return path; } -- GitLab