kargs: parse spaces in kargs input and keep quotes

According to Jonathan's suggestion, should fix the code from
ostree repo.

With this patch:
- kargs input like "init_on_alloc=1 init_on_free=1", will be
parsed as 2 seperated args `init_on_alloc=1` and `init_on_free=1`,
instead of whole;
- According to https://www.kernel.org/doc/html/v4.14/admin-guide/kernel-parameters.html,
need to keep spaces in double-quotes, like `param="spaces in here"`
will be parsed as whole instead of 3.

Fixes https://github.com/coreos/rpm-ostree/issues/4821
This commit is contained in:
HuijingHei 2024-03-04 10:44:42 +08:00
parent d95c2f8dd8
commit abc7d5b9a0
4 changed files with 172 additions and 82 deletions

View File

@ -163,6 +163,44 @@ strcmp0_equal (gconstpointer v1, gconstpointer v2)
return g_strcmp0 (v1, v2) == 0;
}
/* Split string with spaces, but keep quotes.
For example, "test=\"1 2\"" will be saved as whole.
*/
static char **
split_kernel_args (const char *str)
{
gboolean quoted = FALSE;
g_return_val_if_fail (str != NULL, NULL);
GPtrArray *strv = g_ptr_array_new ();
size_t len = strlen (str);
// Skip first spaces
const char *start = str + strspn (str, " ");
for (const char *iter = start; iter && *iter; iter++)
{
if (*iter == '"')
quoted = !quoted;
else if (*iter == ' ' && !quoted)
{
g_ptr_array_add (strv, g_strndup (start, iter - start));
start = iter + 1;
}
}
// Add the last slice
if (!quoted)
g_ptr_array_add (strv, g_strndup (start, str + len - start));
else
{
g_debug ("Missing terminating quote in '%s'.\n", str);
g_assert_false (quoted);
}
g_ptr_array_add (strv, NULL);
return (char **)g_ptr_array_free (strv, FALSE);
}
/**
* ostree_kernel_args_new: (skip)
*
@ -285,35 +323,42 @@ _ostree_kernel_arg_get_key_array (OstreeKernelArgs *kargs)
gboolean
ostree_kernel_args_new_replace (OstreeKernelArgs *kargs, const char *arg, GError **error)
{
g_autofree char *arg_owned = g_strdup (arg);
const char *key = arg_owned;
const char *val = split_keyeq (arg_owned);
// Split the arg
g_auto (GStrv) argv = split_kernel_args (arg);
GPtrArray *entries = g_hash_table_lookup (kargs->table, key);
if (!entries)
return glnx_throw (error, "No key '%s' found", key);
g_assert_cmpuint (entries->len, >, 0);
/* first handle the case where the user just wants to replace an old value */
if (val && strchr (val, '='))
for (char **iter = argv; iter && *iter; iter++)
{
g_autofree char *old_val = g_strdup (val);
const char *new_val = split_keyeq (old_val);
g_assert (new_val);
g_autofree char *arg_owned = g_strdup (*iter);
const char *key = arg_owned;
const char *val = split_keyeq (arg_owned);
guint i = 0;
if (!ot_ptr_array_find_with_equal_func (entries, old_val, kernel_args_entry_value_equal, &i))
return glnx_throw (error, "No karg '%s=%s' found", key, old_val);
GPtrArray *entries = g_hash_table_lookup (kargs->table, key);
if (!entries)
return glnx_throw (error, "No key '%s' found", key);
g_assert_cmpuint (entries->len, >, 0);
kernel_args_entry_replace_value (entries->pdata[i], new_val);
return TRUE;
/* first handle the case where the user just wants to replace an old value */
if (val && strchr (val, '='))
{
g_autofree char *old_val = g_strdup (val);
const char *new_val = split_keyeq (old_val);
g_assert (new_val);
guint i = 0;
if (!ot_ptr_array_find_with_equal_func (entries, old_val, kernel_args_entry_value_equal,
&i))
return glnx_throw (error, "No karg '%s=%s' found", key, old_val);
kernel_args_entry_replace_value (entries->pdata[i], new_val);
continue;
}
/* can't know which val to replace without the old_val=new_val syntax */
if (entries->len > 1)
return glnx_throw (error, "Multiple values for key '%s' found", key);
kernel_args_entry_replace_value (entries->pdata[0], val);
}
/* can't know which val to replace without the old_val=new_val syntax */
if (entries->len > 1)
return glnx_throw (error, "Multiple values for key '%s' found", key);
kernel_args_entry_replace_value (entries->pdata[0], val);
return TRUE;
}
@ -381,39 +426,48 @@ ostree_kernel_args_delete_key_entry (OstreeKernelArgs *kargs, const char *key, G
gboolean
ostree_kernel_args_delete (OstreeKernelArgs *kargs, const char *arg, GError **error)
{
g_autofree char *arg_owned = g_strdup (arg);
const char *key = arg_owned;
const char *val = split_keyeq (arg_owned);
// Split the arg
g_auto (GStrv) argv = split_kernel_args (arg);
GPtrArray *entries = g_hash_table_lookup (kargs->table, key);
if (!entries)
return glnx_throw (error, "No key '%s' found", key);
g_assert_cmpuint (entries->len, >, 0);
/* special-case: we allow deleting by key only if there's only one val */
if (entries->len == 1)
for (char **iter = argv; iter && *iter; iter++)
{
/* but if a specific val was passed, check that it's the same */
OstreeKernelArgsEntry *e = entries->pdata[0];
if (val && !strcmp0_equal (val, _ostree_kernel_args_entry_get_value (e)))
return glnx_throw (error, "No karg '%s=%s' found", key, val);
return ostree_kernel_args_delete_key_entry (kargs, key, error);
}
g_autofree char *arg_owned = g_strdup (*iter);
/* note val might be NULL here, in which case we're looking for `key`, not `key=` or
* `key=val` */
guint i = 0;
if (!ot_ptr_array_find_with_equal_func (entries, val, kernel_args_entry_value_equal, &i))
{
if (!val)
/* didn't find NULL -> only key= key=val1 key=val2 style things left, so the user
* needs to be more specific */
return glnx_throw (error, "Multiple values for key '%s' found", arg);
return glnx_throw (error, "No karg '%s' found", arg);
}
const char *key = arg_owned;
const char *val = split_keyeq (arg_owned);
g_assert (g_ptr_array_remove (kargs->order, entries->pdata[i]));
g_assert (g_ptr_array_remove_index (entries, i));
GPtrArray *entries = g_hash_table_lookup (kargs->table, key);
if (!entries)
return glnx_throw (error, "No key '%s' found", key);
g_assert_cmpuint (entries->len, >, 0);
/* special-case: we allow deleting by key only if there's only one val */
if (entries->len == 1)
{
/* but if a specific val was passed, check that it's the same */
OstreeKernelArgsEntry *e = entries->pdata[0];
if (val && !strcmp0_equal (val, _ostree_kernel_args_entry_get_value (e)))
return glnx_throw (error, "No karg '%s=%s' found", key, val);
if (!ostree_kernel_args_delete_key_entry (kargs, key, error))
return glnx_throw (error, "Remove key entry '%s=%s' failed.", key, val);
continue;
}
/* note val might be NULL here, in which case we're looking for `key`, not `key=` or
* `key=val` */
guint i = 0;
if (!ot_ptr_array_find_with_equal_func (entries, val, kernel_args_entry_value_equal, &i))
{
if (!val)
/* didn't find NULL -> only key= key=val1 key=val2 style things left, so the user
* needs to be more specific */
return glnx_throw (error, "Multiple values for key '%s' found", arg);
return glnx_throw (error, "No karg '%s' found", arg);
}
g_assert (g_ptr_array_remove (kargs->order, entries->pdata[i]));
g_assert (g_ptr_array_remove_index (entries, i));
}
return TRUE;
}
@ -499,27 +553,34 @@ ostree_kernel_args_replace (OstreeKernelArgs *kargs, const char *arg)
void
ostree_kernel_args_append (OstreeKernelArgs *kargs, const char *arg)
{
gboolean existed = TRUE;
GPtrArray *entries = NULL;
char *duped = g_strdup (arg);
const char *val = split_keyeq (duped);
// Split the arg
g_auto (GStrv) argv = split_kernel_args (arg);
entries = g_hash_table_lookup (kargs->table, duped);
if (!entries)
for (char **iter = argv; iter && *iter; iter++)
{
entries = g_ptr_array_new_with_free_func (kernel_args_entry_free_from_table);
existed = FALSE;
gboolean existed = TRUE;
GPtrArray *entries = NULL;
char *duped = g_strdup (*iter);
const char *val = split_keyeq (duped);
entries = g_hash_table_lookup (kargs->table, duped);
if (!entries)
{
entries = g_ptr_array_new_with_free_func (kernel_args_entry_free_from_table);
existed = FALSE;
}
OstreeKernelArgsEntry *entry = _ostree_kernel_args_entry_new ();
_ostree_kernel_args_entry_set_key (entry, duped);
_ostree_kernel_args_entry_set_value (entry, g_strdup (val));
g_ptr_array_add (entries, entry);
g_ptr_array_add (kargs->order, entry);
if (!existed)
g_hash_table_replace (kargs->table, duped, entries);
}
OstreeKernelArgsEntry *entry = _ostree_kernel_args_entry_new ();
_ostree_kernel_args_entry_set_key (entry, duped);
_ostree_kernel_args_entry_set_value (entry, g_strdup (val));
g_ptr_array_add (entries, entry);
g_ptr_array_add (kargs->order, entry);
if (!existed)
g_hash_table_replace (kargs->table, duped, entries);
}
/**
@ -644,7 +705,7 @@ ostree_kernel_args_parse_append (OstreeKernelArgs *kargs, const char *options)
if (!options)
return;
args = g_strsplit (options, " ", -1);
args = split_kernel_args (options);
for (iter = args; *iter; iter++)
{
char *arg = *iter;

View File

@ -184,10 +184,7 @@ ot_admin_builtin_deploy (int argc, char **argv, OstreeCommandInvocation *invocat
&& (opt_kernel_argv || opt_kernel_argv_append || opt_kernel_argv_delete))
{
OstreeBootconfigParser *bootconfig = ostree_deployment_get_bootconfig (merge_deployment);
g_auto (GStrv) previous_args
= g_strsplit (ostree_bootconfig_parser_get (bootconfig, "options"), " ", -1);
kargs = ostree_kernel_args_new ();
ostree_kernel_args_append_argv (kargs, previous_args);
kargs = ostree_kernel_args_from_string (ostree_bootconfig_parser_get (bootconfig, "options"));
}
/* Now replace/extend the above set. Note that if no options are specified,

View File

@ -24,7 +24,7 @@ set -euo pipefail
# Exports OSTREE_SYSROOT so --sysroot not needed.
setup_os_repository "archive" "syslinux"
echo "1..4"
echo "1..5"
${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmain/x86_64-runtime
rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmain/x86_64-runtime)
@ -77,3 +77,10 @@ assert_not_file_has_content sysroot/boot/loader/entries/ostree-2.conf 'options.*
assert_not_file_has_content sysroot/boot/loader/entries/ostree-2.conf 'options.*APPENDARG=VALAPPEND'
echo "ok deploy --karg-delete"
${CMD_PREFIX} ostree admin deploy --os=testos --karg-append 'test="1 2"' testos:testos/buildmain/x86_64-runtime
assert_file_has_content sysroot/boot/loader/entries/ostree-2.conf 'options.*test="1 2"'
${CMD_PREFIX} ostree admin deploy --os=testos --karg-delete 'test="1 2"' testos:testos/buildmain/x86_64-runtime
assert_not_file_has_content sysroot/boot/loader/entries/ostree-2.conf 'options.*test="1 2"'
echo "ok deploy --karg-delete with quotes"

View File

@ -24,8 +24,7 @@
static gboolean
check_string_existance (OstreeKernelArgs *karg, const char *string_to_find)
{
g_autofree gchar *string_with_spaces = ostree_kernel_args_to_string (karg);
g_auto (GStrv) string_list = g_strsplit (string_with_spaces, " ", -1);
g_auto (GStrv) string_list = ostree_kernel_args_to_strv (karg);
return g_strv_contains ((const char *const *)string_list, string_to_find);
}
@ -140,6 +139,16 @@ test_kargs_delete (void)
g_assert_no_error (error);
g_assert (ret);
g_assert (!check_string_existance (karg, "nosmt"));
/* Make sure we also support quotes and spaces */
ostree_kernel_args_append (karg, "foo=\"1 2\" bar=0");
check_string_existance (karg, "foo=\"1 2\"");
check_string_existance (karg, "bar=0");
ret = ostree_kernel_args_delete (karg, "foo=\"1 2\" bar=0", &error);
g_assert_no_error (error);
g_assert (ret);
g_assert (!check_string_existance (karg, "foo=\"1 2\""));
g_assert (!check_string_existance (karg, "bar=0"));
}
static void
@ -189,6 +198,16 @@ test_kargs_replace (void)
g_assert (ret);
g_assert (!check_string_existance (karg, "test=firstval"));
g_assert (check_string_existance (karg, "test=newval"));
/* Replace with input contains quotes and spaces, it should support */
ostree_kernel_args_append (karg, "foo=1 bar=\"1 2\"");
ret = ostree_kernel_args_new_replace (karg, "foo=\"1 2\" bar", &error);
g_assert_no_error (error);
g_assert (ret);
g_assert (!check_string_existance (karg, "foo=1"));
g_assert (!check_string_existance (karg, "bar=\"1 2\""));
g_assert (check_string_existance (karg, "foo=\"1 2\""));
g_assert (check_string_existance (karg, "bar"));
}
/* In this function, we want to verify that ostree_kernel_args_append
@ -206,6 +225,7 @@ test_kargs_append (void)
ostree_kernel_args_append (append_arg, "test=");
ostree_kernel_args_append (append_arg, "test");
ostree_kernel_args_append (append_arg, "second_test");
ostree_kernel_args_append (append_arg, "second_test=0 second_test=\"1 2\"");
/* We loops through the kargs inside table to verify
* the functionality of append because at this stage
@ -230,6 +250,10 @@ test_kargs_append (void)
g_assert_cmpstr (key, ==, "second_test");
g_assert (ot_ptr_array_find_with_equal_func (value_array, NULL,
kernel_args_entry_value_equal, NULL));
g_assert (ot_ptr_array_find_with_equal_func (value_array, "0",
kernel_args_entry_value_equal, NULL));
g_assert (ot_ptr_array_find_with_equal_func (value_array, "\"1 2\"",
kernel_args_entry_value_equal, NULL));
}
}
@ -243,14 +267,15 @@ test_kargs_append (void)
/* Up till this point, we verified that the above was all correct, we then
* check ostree_kernel_args_to_string has the right result
*/
g_autofree gchar *kargs_str = ostree_kernel_args_to_string (append_arg);
g_auto (GStrv) kargs_list = g_strsplit (kargs_str, " ", -1);
g_auto (GStrv) kargs_list = ostree_kernel_args_to_strv (append_arg);
g_assert (g_strv_contains ((const char *const *)kargs_list, "test=valid"));
g_assert (g_strv_contains ((const char *const *)kargs_list, "test=secondvalid"));
g_assert (g_strv_contains ((const char *const *)kargs_list, "test="));
g_assert (g_strv_contains ((const char *const *)kargs_list, "test"));
g_assert (g_strv_contains ((const char *const *)kargs_list, "second_test"));
g_assert_cmpint (5, ==, g_strv_length (kargs_list));
g_assert (g_strv_contains ((const char *const *)kargs_list, "second_test=0"));
g_assert (g_strv_contains ((const char *const *)kargs_list, "second_test=\"1 2\""));
g_assert_cmpint (7, ==, g_strv_length (kargs_list));
}
int