Skip to content

Commit

Permalink
Merge pull request #3208 from HuijingHei/split-whitespace
Browse files Browse the repository at this point in the history 
kargs: parse spaces in kargs input and keep quotes
  • Loading branch information
@HuijingHei
HuijingHei authored Mar 11, 2024
2 parents d95c2f8 + abc7d5b commit 223a1af
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 79 deletions.
199 changes: 130 additions & 69 deletions src/libostree/ostree-kernel-args.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
*
Expand Down Expand Up @@ -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);

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);
// Split the arg
g_auto (GStrv) argv = split_kernel_args (arg);

/* 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);
/* 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);
kernel_args_entry_replace_value (entries->pdata[0], val);
}
return TRUE;
}

Expand Down Expand Up @@ -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);

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);
// Split the arg
g_auto (GStrv) argv = split_kernel_args (arg);

/* 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);

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;
}

g_assert (g_ptr_array_remove (kargs->order, entries->pdata[i]));
g_assert (g_ptr_array_remove_index (entries, i));
/* 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;
}

Expand Down Expand Up @@ -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;

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));
char *duped = g_strdup (*iter);
const char *val = split_keyeq (duped);

g_ptr_array_add (entries, entry);
g_ptr_array_add (kargs->order, entry);
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));

if (!existed)
g_hash_table_replace (kargs->table, duped, entries);
g_ptr_array_add (entries, entry);
g_ptr_array_add (kargs->order, entry);

if (!existed)
g_hash_table_replace (kargs->table, duped, entries);
}
}

/**
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 1 addition & 4 deletions src/ostree/ot-admin-builtin-deploy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 8 additions & 1 deletion tests/test-admin-deploy-karg.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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"
35 changes: 30 additions & 5 deletions tests/test-kargs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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));
}
}

Expand All @@ -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
Expand Down

0 comments on commit 223a1af

Please sign in to comment.