From ebc972928f32c5708ea437732740aabcd618571a Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Thu, 30 Jan 2025 20:05:12 +0000 Subject: [PATCH] pv-adverb: Factor out parsing of --ld-preload, --ld-audit arguments This allows it to have test coverage. Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/adverb-preload.c | 78 ++++++++++++++++ pressure-vessel/adverb-preload.h | 6 ++ pressure-vessel/adverb.c | 54 ++--------- tests/pressure-vessel/adverb-preload.c | 124 +++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 47 deletions(-) diff --git a/pressure-vessel/adverb-preload.c b/pressure-vessel/adverb-preload.c index 2685f6fbf..f6b8de831 100644 --- a/pressure-vessel/adverb-preload.c +++ b/pressure-vessel/adverb-preload.c @@ -183,3 +183,81 @@ pv_adverb_set_up_preload_modules (FlatpakBwrap *wrapped_command, return TRUE; } + +/* + * pv_adverb_preload_module_parse_adverb_cli: + * @self: The module, which must be initialized + * to %PV_ADVERB_PRELOAD_MODULE_INIT + * @option: The command-line option that we are parsing, for + * example `--ld-preload` + * @which: The corresponding type of module + * @value: The argument to the command-line option + * @error: Used to report an error if unsuccessful + * + * Parse the `--ld-preload` or `--ld-audit` option of `pv-adverb`. + * + * Returns: %TRUE if the argument was parsed successfully + */ +gboolean +pv_adverb_preload_module_parse_adverb_cli (PvAdverbPreloadModule *self, + const char *option, + PvPreloadVariableIndex which, + const char *value, + GError **error) +{ + g_auto(GStrv) parts = NULL; + const char *architecture = NULL; + gsize abi_index = PV_UNSPECIFIED_ABI; + gsize abi; + + g_return_val_if_fail (self->name == NULL, FALSE); + g_return_val_if_fail (self->abi_index == PV_UNSPECIFIED_ABI, FALSE); + + parts = g_strsplit (value, ":", 0); + + if (parts[0] != NULL) + { + gsize i; + + for (i = 1; parts[i] != NULL; i++) + { + if (g_str_has_prefix (parts[i], "abi=")) + { + architecture = parts[i] + strlen ("abi="); + + for (abi = 0; abi < PV_N_SUPPORTED_ARCHITECTURES; abi++) + { + if (strcmp (architecture, pv_multiarch_tuples[abi]) == 0) + { + abi_index = abi; + break; + } + } + + if (abi_index == PV_UNSPECIFIED_ABI) + { + g_set_error (error, G_OPTION_ERROR, G_OPTION_ERROR_BAD_VALUE, + "Unsupported ABI %s", + architecture); + return FALSE; + } + + continue; + } + else + { + g_set_error (error, G_OPTION_ERROR, G_OPTION_ERROR_BAD_VALUE, + "Unexpected option in %s=\"%s\": %s", + option, value, parts[i]); + return FALSE; + } + } + + value = parts[0]; + } + + self->index_in_preload_variables = which; + self->name = g_strdup (value); + self->abi_index = abi_index; + return TRUE; +} diff --git a/pressure-vessel/adverb-preload.h b/pressure-vessel/adverb-preload.h index bd9c578dc..0e76b9e77 100644 --- a/pressure-vessel/adverb-preload.h +++ b/pressure-vessel/adverb-preload.h @@ -56,6 +56,12 @@ pv_adverb_preload_module_clear (gpointer p) G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC (PvAdverbPreloadModule, pv_adverb_preload_module_clear) +gboolean pv_adverb_preload_module_parse_adverb_cli (PvAdverbPreloadModule *self, + const char *option, + PvPreloadVariableIndex which, + const char *value, + GError **error); + gboolean pv_adverb_set_up_preload_modules (FlatpakBwrap *wrapped_command, PvPerArchDirs *lib_temp_dirs, const PvAdverbPreloadModule *preload_modules, diff --git a/pressure-vessel/adverb.c b/pressure-vessel/adverb.c index bd9499d0c..e2f99497e 100644 --- a/pressure-vessel/adverb.c +++ b/pressure-vessel/adverb.c @@ -128,52 +128,14 @@ opt_ld_something (const char *option, gpointer data, GError **error) { - PvAdverbPreloadModule module = { NULL, 0, PV_UNSPECIFIED_ABI }; - g_auto(GStrv) parts = NULL; - const char *architecture = NULL; + PvAdverbPreloadModule module = PV_ADVERB_PRELOAD_MODULE_INIT; - parts = g_strsplit (value, ":", 0); - - if (parts[0] != NULL) - { - gsize i; - - for (i = 1; parts[i] != NULL; i++) - { - if (g_str_has_prefix (parts[i], "abi=")) - { - gsize abi; - - architecture = parts[i] + strlen ("abi="); - - for (abi = 0; abi < PV_N_SUPPORTED_ARCHITECTURES; abi++) - { - if (strcmp (architecture, pv_multiarch_details[abi].tuple) == 0) - { - module.abi_index = abi; - break; - } - } - - if (module.abi_index == PV_UNSPECIFIED_ABI) - { - g_set_error (error, G_OPTION_ERROR, G_OPTION_ERROR_BAD_VALUE, - "Unsupported ABI %s", - architecture); - return FALSE; - } - } - else - { - g_set_error (error, G_OPTION_ERROR, G_OPTION_ERROR_BAD_VALUE, - "Unexpected option in %s=\"%s\": %s", - option, value, parts[i]); - return FALSE; - } - } - - value = parts[0]; - } + if (!pv_adverb_preload_module_parse_adverb_cli (&module, + option, + index_in_preload_variables, + value, + error)) + return FALSE; if (opt_preload_modules == NULL) { @@ -181,8 +143,6 @@ opt_ld_something (const char *option, g_array_set_clear_func (opt_preload_modules, pv_adverb_preload_module_clear); } - module.index_in_preload_variables = index_in_preload_variables; - module.name = g_strdup (value); g_array_append_val (opt_preload_modules, module); return TRUE; } diff --git a/tests/pressure-vessel/adverb-preload.c b/tests/pressure-vessel/adverb-preload.c index a50fc2a01..744a91805 100644 --- a/tests/pressure-vessel/adverb-preload.c +++ b/tests/pressure-vessel/adverb-preload.c @@ -310,6 +310,126 @@ test_biarch (Fixture *f, #endif } +typedef struct +{ + const char *option; + PvAdverbPreloadModule expected; +} CommandLineTest; + +static void +test_cli (Fixture *f, + gconstpointer context) +{ + static const CommandLineTest tests[] = + { + { + .option = "", + .expected = { + .name = (char *) "", + .abi_index = PV_UNSPECIFIED_ABI, + }, + }, + { + .option = "libpreload.so", + .expected = { + .name = (char *) "libpreload.so", + .abi_index = PV_UNSPECIFIED_ABI, + }, + }, +#if defined(__x86_64__) || defined(__i386__) + { + .option = "/lib64/libpreload.so:abi=" SRT_ABI_X86_64, + .expected = { + .name = (char *) "/lib64/libpreload.so", + .abi_index = 0, + }, + }, + { + .option = "/lib32/libpreload.so:abi=" SRT_ABI_I386, + .expected = { + .name = (char *) "/lib32/libpreload.so", + .abi_index = 1, + }, + }, +#endif +#if defined(_SRT_MULTIARCH) + { + .option = "/tmp/libabi.so:abi=" _SRT_MULTIARCH, + .expected = { + .name = (char *) "/tmp/libabi.so", +#if defined(__i386__) + /* On i386, we treat x86_64 as the primary architecture */ + .abi_index = 1, +#else + .abi_index = 0, +#endif + }, + }, +#endif + { + .option = "/tmp/libabi.so:nonsense", + .expected = { + .name = NULL, + }, + }, + }; + gsize i; + + for (i = 0; i < G_N_ELEMENTS (tests); i++) + { + const CommandLineTest *test = &tests[i]; + /* There's no real difference between our handling of LD_AUDIT + * and LD_PRELOAD, so we alternate between testing them both */ + const PvPreloadVariableIndex which = i % 2; + const char *option = pv_preload_variables[which].adverb_option; + g_autoptr(GError) local_error = NULL; + g_auto(PvAdverbPreloadModule) actual = PV_ADVERB_PRELOAD_MODULE_INIT; + gboolean ret; + + ret = pv_adverb_preload_module_parse_adverb_cli (&actual, + option, + which, + test->option, + &local_error); + + if (ret) + { + const char *abi = "(unspecified)"; + + if (actual.abi_index != PV_UNSPECIFIED_ABI) + { + g_assert_cmpuint (actual.abi_index, >=, 0); + g_assert_cmpuint (actual.abi_index, <, PV_N_SUPPORTED_ARCHITECTURES); + abi = pv_multiarch_details[actual.abi_index].tuple; + } + + g_test_message ("\"%s\" -> \"%s\", abi=%s", + test->option, actual.name, abi); + } + else + { + g_test_message ("\"%s\" -> error \"%s\"", + test->option, local_error->message); + } + + if (test->expected.name == NULL) + { + g_assert_false (ret); + g_assert_null (actual.name); + g_assert_nonnull (local_error); + } + else + { + g_assert_no_error (local_error); + g_assert_true (ret); + g_assert_cmpstr (actual.name, ==, test->expected.name); + g_assert_cmpuint (actual.abi_index, ==, test->expected.abi_index); + g_assert_cmpuint (actual.index_in_preload_variables, ==, which); + pv_adverb_preload_module_clear (&actual); + } + } +} + /* * There is a special case for gameoverlayrenderer.so: * pv-adverb --ld-preload=/.../ubuntu12_32/gameoverlayrenderer.so is @@ -706,5 +826,9 @@ main (int argc, add_test ("/gameoverlayrenderer", test_gameoverlayrenderer); add_test ("/repetition", test_repetition); + /* This one isn't affected by whether we have the PvPerArchDirs or not */ + g_test_add ("/cli", Fixture, NULL, + setup, test_cli, teardown); + return g_test_run (); } -- GitLab