[PATCH v3 1/2] discover: Allow for empty paths

Klaus Heinrich Kiwi klaus at linux.vnet.ibm.com
Thu Jan 21 06:26:57 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.

Signed-off-by: Klaus Heinrich Kiwi <klaus at linux.vnet.ibm.com>
---
 discover/grub2/builtins.c        | 10 ++++++----
 discover/grub2/grub2.c           | 29 +++++++++++++++++++----------
 discover/paths.c                 | 16 +++++++++++-----
 discover/paths.h                 |  6 +++---
 test/parser/test-grub2-devpath.c | 28 +++++++++++++++++++---------
 test/parser/utils.c              |  2 +-
 6 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/discover/grub2/builtins.c b/discover/grub2/builtins.c
index ab1407a..4b94f99 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,9 @@ 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..7fb6f62 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);
@@ -118,7 +119,6 @@ struct grub2_file *grub2_parse_file(struct grub2_script *script,
 		const char *str)
 {
 	struct grub2_file *file;
-	size_t dev_len;
 	char *pos;
 
 	if (!str)
@@ -130,24 +130,33 @@ struct grub2_file *grub2_parse_file(struct grub2_script *script,
 		/* just a path - no device, return path as-is */
 		file->path = talloc_strdup(file, str);
 
-	} else {
+	}
+	else {
 		/* device plus path - split into components */
-
 		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);
+		/* same as above for device string */
+		if ( pos > (str+1) )
+			file->dev = talloc_strndup(file, str + 1,
+				       (size_t) (pos - str - 1));
 	}
 
+	/* 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/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