[PATCH v4 1/2] discover: Allow for empty paths
Klaus Heinrich Kiwi
klaus at linux.vnet.ibm.com
Fri Jan 22 05:44:36 AEDT 2021
Some grub tests involve a (device)/path structure, where it is
actually legal to have an empty path. Adjust join_path() and
dependant functions to allow for empty pathnames.
Also add / modify a few more tests for this scenario.
Signed-off-by: Klaus Heinrich Kiwi <klaus at linux.vnet.ibm.com>
---
discover/grub2/builtins.c | 9 +++++----
discover/grub2/grub2.c | 17 ++++++++++++----
discover/paths.c | 16 ++++++++++-----
discover/paths.h | 6 +++---
test/parser/test-grub2-devpath.c | 28 +++++++++++++++++---------
test/parser/test-grub2-test-file-ops.c | 19 +++++++++++++++++
test/parser/utils.c | 2 +-
7 files changed, 71 insertions(+), 26 deletions(-)
diff --git a/discover/grub2/builtins.c b/discover/grub2/builtins.c
index ab1407a..f32e8e5 100644
--- a/discover/grub2/builtins.c
+++ b/discover/grub2/builtins.c
@@ -247,13 +247,13 @@ static bool builtin_test_op_file(struct grub2_script *script, char op,
switch (op) {
case 's':
/* -s: return true if file exists and has non-zero size */
- result = statbuf.st_size > 0;
+ result = !path ? false : statbuf.st_size > 0;
break;
case 'f':
/* -f: return true if file exists and is not a directory. This is
* different than the behavior of "test", but is what GRUB does
* (though note as above that we follow symlinks unlike GRUB). */
- result = !S_ISDIR(statbuf.st_mode);
+ result = !path ? false : !S_ISDIR(statbuf.st_mode);
break;
default:
result = false;
@@ -284,7 +284,7 @@ static bool builtin_test_op_dir(struct grub2_script *script, char op,
if (rc)
return false;
- return S_ISDIR(statbuf.st_mode);
+ return !path ? false : S_ISDIR(statbuf.st_mode);
}
static bool builtin_test_op(struct grub2_script *script,
@@ -419,7 +419,8 @@ static int builtin_source(struct grub2_script *script,
return false;
rc = parse_to_device_path(script, argv[1], &dev, &path);
- if (rc)
+ /* We need to have a valid (non-empty) path for sources */
+ if (rc || !path)
return false;
rc = parser_request_file(script->ctx, dev, path, &buf, &len);
diff --git a/discover/grub2/grub2.c b/discover/grub2/grub2.c
index b176ce2..9be94eb 100644
--- a/discover/grub2/grub2.c
+++ b/discover/grub2/grub2.c
@@ -70,7 +70,8 @@ struct resource *create_grub2_resource(struct grub2_script *script,
}
file = grub2_parse_file(script, path);
- if (!file)
+ /* We need a non-empty path for resources */
+ if (!file || !file->path)
return NULL;
res = talloc(opt, struct resource);
@@ -135,19 +136,27 @@ struct grub2_file *grub2_parse_file(struct grub2_script *script,
pos = strchr(str, ')');
- /* no closing bracket, or zero-length path? */
- if (!pos || *(pos+1) == '\0') {
+ /* no closing bracket is not legal */
+ if (!pos) {
talloc_free(file);
return NULL;
}
- file->path = talloc_strdup(file, pos + 1);
+ /* path is non-empty - copy it (otherwise keep it
+ * NULL as it should be legal) */
+ if (*(pos+1) != '\0')
+ file->path = talloc_strdup(file, pos + 1);
dev_len = pos - str - 1;
if (dev_len)
file->dev = talloc_strndup(file, str + 1, dev_len);
}
+ /* if both dev and path are empty, this is probably an error */
+ if (!file->dev && !file->path) {
+ talloc_free(file);
+ return NULL;
+ }
return file;
}
diff --git a/discover/paths.c b/discover/paths.c
index 16fdd59..3c43bf6 100644
--- a/discover/paths.c
+++ b/discover/paths.c
@@ -51,13 +51,19 @@ const char *mount_base(void)
char *join_paths(void *alloc_ctx, const char *a, const char *b)
{
char *full_path;
+ size_t a_len = a ? strlen(a) : 0;
+ size_t b_len = b ? strlen(b) : 0;
- full_path = talloc_array(alloc_ctx, char, strlen(a) + strlen(b) + 2);
+ full_path = talloc_zero_array(alloc_ctx, char, a_len + b_len + 2);
- strcpy(full_path, a);
- if (b[0] != '/' && a[strlen(a) - 1] != '/')
- strcat(full_path, "/");
- strcat(full_path, b);
+ if (a_len)
+ strcpy(full_path, a);
+
+ if (b_len) {
+ if (a_len && a[a_len - 1] != '/' && b[0] != '/')
+ strcat(full_path, "/");
+ strcat(full_path, b);
+ }
return full_path;
}
diff --git a/discover/paths.h b/discover/paths.h
index 67fe8a3..6694877 100644
--- a/discover/paths.h
+++ b/discover/paths.h
@@ -6,10 +6,10 @@
#include <process/process.h>
/**
- * Utility function for joining two paths. Adds a / between a and b if
- * required.
+ * Utility function for joining two paths. Allows either or
+ * both paths to be NULL. Adds a / between a and b if required.
*
- * Returns a newly-allocated string.
+ * Returns a newly-allocated string (empty if both paths NULL)
*/
char *join_paths(void *alloc_ctx, const char *a, const char *b);
diff --git a/test/parser/test-grub2-devpath.c b/test/parser/test-grub2-devpath.c
index d1d00f1..2571078 100644
--- a/test/parser/test-grub2-devpath.c
+++ b/test/parser/test-grub2-devpath.c
@@ -9,35 +9,40 @@ menuentry a {
linux /vmlinux
}
+# local, with an empty device-string
+menuentry b {
+ linux ()/vmlinux
+}
+
# local, specified by root env var
root=00000000-0000-0000-0000-000000000001
-menuentry b {
+menuentry c {
linux /vmlinux
}
# remote, specified by root env var
root=00000000-0000-0000-0000-000000000002
-menuentry c {
+menuentry d {
linux /vmlinux
}
# local, full dev+path spec
-menuentry d {
+menuentry e {
linux (00000000-0000-0000-0000-000000000001)/vmlinux
}
# remote, full dev+path spec
-menuentry e {
+menuentry f {
linux (00000000-0000-0000-0000-000000000002)/vmlinux
}
# invalid: incomplete dev+path spec
-menuentry f {
+menuentry g {
linux (00000000-0000-0000-0000-000000000001
}
# invalid: no path
-menuentry g {
+menuentry h {
linux (00000000-0000-0000-0000-000000000001)
}
@@ -64,7 +69,7 @@ void run_test(struct parser_test *test)
test_run_parser(test, "grub2");
- check_boot_option_count(ctx, 5);
+ check_boot_option_count(ctx, 6);
opt = get_boot_option(ctx, 0);
check_name(opt, "a");
@@ -76,13 +81,18 @@ void run_test(struct parser_test *test)
opt = get_boot_option(ctx, 2);
check_name(opt, "c");
- check_resolved_local_resource(opt->boot_image, dev2, "/vmlinux");
+ check_resolved_local_resource(opt->boot_image, dev1, "/vmlinux");
opt = get_boot_option(ctx, 3);
check_name(opt, "d");
- check_resolved_local_resource(opt->boot_image, dev1, "/vmlinux");
+ check_resolved_local_resource(opt->boot_image, dev2, "/vmlinux");
opt = get_boot_option(ctx, 4);
check_name(opt, "e");
+ check_resolved_local_resource(opt->boot_image, dev1, "/vmlinux");
+
+ opt = get_boot_option(ctx, 5);
+ check_name(opt, "f");
check_resolved_local_resource(opt->boot_image, dev2, "/vmlinux");
+
}
diff --git a/test/parser/test-grub2-test-file-ops.c b/test/parser/test-grub2-test-file-ops.c
index b614fc1..38214a1 100644
--- a/test/parser/test-grub2-test-file-ops.c
+++ b/test/parser/test-grub2-test-file-ops.c
@@ -13,6 +13,12 @@ fi
if [ ! -f /empty_file -a $status = success ]
then status=fail_f_3
fi
+if [ -f "" -a $status = success ]
+then status=fail_f_4
+fi
+if [ -f / -a $status = success ]
+then status=fail_f_5
+fi
if [ -s /file_that_does_not_exist -a $status = success ]
then status=fail_s_1
@@ -26,6 +32,12 @@ fi
if [ ! -s /non_empty_file -a $status = success ]
then status=fail_s_4
fi
+if [ -s "" -a $status = success ]
+then status=fail_s_5
+fi
+if [ ! -s / -a $status = success ]
+then status=fail_s_6
+fi
if [ -d /file_that_does_not_exist -a $status = success ]
then status=fail_d_1
@@ -36,6 +48,12 @@ fi
if [ -d /empty_file -a $status = success ]
then status=fail_d_3
fi
+if [ -d "" -a $status = success ]
+then status=fail_d_4
+fi
+if [ ! -d / -a $status = success ]
+then status=fail_d_5
+fi
menuentry $status {
linux /vmlinux
@@ -50,6 +68,7 @@ void run_test(struct parser_test *test)
ctx = test->ctx;
test_read_conf_embedded(test, "/grub2/grub.cfg");
+ test_add_dir(test, ctx->device, "/");
test_add_file_data(test, ctx->device, "/empty_file", "", 0);
test_add_file_data(test, ctx->device, "/non_empty_file", "1", 1);
test_add_dir(test, ctx->device, "/dir");
diff --git a/test/parser/utils.c b/test/parser/utils.c
index d8499a4..2705b9a 100644
--- a/test/parser/utils.c
+++ b/test/parser/utils.c
@@ -257,7 +257,7 @@ int parser_stat_path(struct discover_context *ctx,
list_for_each_entry(&test->files, file, list) {
if (file->dev != dev)
continue;
- if (strcmp(file->name, path))
+ if (path && strcmp(file->name, path))
continue;
statbuf->st_size = (off_t)file->size;
--
2.17.1
More information about the Petitboot
mailing list