[Concept,25/32] boot: pxe: Convert fdtoverlays from string to alist

Message ID 20260109231151.4056804-26-sjg@u-boot.org
State New
Headers
Series boot: pxe: Refactor into separate load/setup phases |

Commit Message

Simon Glass Jan. 9, 2026, 11:11 p.m. UTC
  From: Simon Glass <simon.glass@canonical.com>

Change fdtoverlays from a space-separated string to an alist of
individual paths. This moves the string parsing from boot time to
config-file parsing time, simplifying label_boot_fdtoverlay().

The alist is initialised in label_create() and each overlay path is
added during parsing by the new parse_fdtoverlays() function. The
paths are freed in label_destroy().

Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
---

 boot/pxe_parse.c    | 53 ++++++++++++++++++++++++++++++++++++++++++---
 boot/pxe_utils.c    | 50 ++++++++++--------------------------------
 include/pxe_utils.h |  4 ++--
 test/boot/pxe.c     | 18 +++++++++------
 4 files changed, 75 insertions(+), 50 deletions(-)
  

Patch

diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c
index 72a672354b5..b115fb413a2 100644
--- a/boot/pxe_parse.c
+++ b/boot/pxe_parse.c
@@ -106,12 +106,15 @@  static struct pxe_label *label_create(void)
 	if (!label)
 		return NULL;
 	memset(label, 0, sizeof(struct pxe_label));
+	alist_init_struct(&label->fdtoverlays, char *);
 
 	return label;
 }
 
 void label_destroy(struct pxe_label *label)
 {
+	char **overlayp;
+
 	free(label->name);
 	free(label->kernel_label);
 	free(label->kernel);
@@ -120,7 +123,9 @@  void label_destroy(struct pxe_label *label)
 	free(label->initrd);
 	free(label->fdt);
 	free(label->fdtdir);
-	free(label->fdtoverlays);
+	alist_for_each(overlayp, &label->fdtoverlays)
+		free(*overlayp);
+	alist_uninit(&label->fdtoverlays);
 	free(label->say);
 	free(label);
 }
@@ -305,6 +310,48 @@  static int parse_sliteral(char **c, char **dst)
 	return 1;
 }
 
+/*
+ * Parse a space-separated list of overlay paths into an alist.
+ */
+static int parse_fdtoverlays(char **c, struct alist *overlays)
+{
+	char *val;
+	int err;
+
+	err = parse_sliteral(c, &val);
+	if (err < 0)
+		return err;
+
+	while (*val) {
+		char *path;
+		char *end;
+
+		/* Skip leading spaces */
+		while (*val == ' ')
+			val++;
+
+		if (!*val)
+			break;
+
+		/* Find end of this path */
+		end = strchr(val, ' ');
+		if (end) {
+			path = strndup(val, end - val);
+			val = end;
+		} else {
+			path = strdup(val);
+			val += strlen(val);
+		}
+
+		if (!path || !alist_add(overlays, path)) {
+			free(path);
+			return -ENOMEM;
+		}
+	}
+
+	return 1;
+}
+
 /*
  * Parse a base 10 (unsigned) integer and store it at *dst.
  */
@@ -527,8 +574,8 @@  static int parse_label(char **c, struct pxe_menu *cfg)
 				err = parse_sliteral(c, &label->fdtdir);
 			break;
 		case T_FDTOVERLAYS:
-			if (!label->fdtoverlays)
-				err = parse_sliteral(c, &label->fdtoverlays);
+			if (!label->fdtoverlays.count)
+				err = parse_fdtoverlays(c, &label->fdtoverlays);
 			break;
 		case T_LOCALBOOT:
 			label->localboot = 1;
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index b269ef13f5a..e368b15e500 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -306,37 +306,18 @@  static void label_boot_kaslrseed(struct pxe_context *ctx)
 static void label_boot_fdtoverlay(struct pxe_context *ctx,
 				  struct pxe_label *label)
 {
-	char *fdtoverlay = label->fdtoverlays;
+	struct fdt_header *blob;
+	char **overlayp;
+	ulong addr;
 	int err;
 
 	err = fdt_check_header(ctx->fdt);
 	if (err)
 		return;
 
-	/* Cycle over the overlay files and apply them in order */
-	do {
-		struct fdt_header *blob;
-		char *overlayfile;
-		ulong addr;
-		char *end;
-		int len;
-
-		/* Drop leading spaces */
-		while (*fdtoverlay == ' ')
-			++fdtoverlay;
-
-		/* Copy a single filename if multiple provided */
-		end = strstr(fdtoverlay, " ");
-		if (end) {
-			len = (int)(end - fdtoverlay);
-			overlayfile = malloc(len + 1);
-			strncpy(overlayfile, fdtoverlay, len);
-			overlayfile[len] = '\0';
-		} else
-			overlayfile = fdtoverlay;
-
-		if (!strlen(overlayfile))
-			goto skip_overlay;
+	/* Apply each overlay file in order */
+	alist_for_each(overlayp, &label->fdtoverlays) {
+		const char *overlayfile = *overlayp;
 
 		/* Load overlay file */
 		err = get_relfile_envaddr(ctx, overlayfile, "fdtoverlay_addr_r",
@@ -345,7 +326,7 @@  static void label_boot_fdtoverlay(struct pxe_context *ctx,
 					  &addr, NULL);
 		if (err < 0) {
 			printf("Failed loading overlay %s\n", overlayfile);
-			goto skip_overlay;
+			continue;
 		}
 
 		/* Resize main fdt */
@@ -354,22 +335,15 @@  static void label_boot_fdtoverlay(struct pxe_context *ctx,
 		blob = map_sysmem(addr, 0);
 		err = fdt_check_header(blob);
 		if (err) {
-			printf("Invalid overlay %s, skipping\n",
-			       overlayfile);
-			goto skip_overlay;
+			printf("Invalid overlay %s, skipping\n", overlayfile);
+			continue;
 		}
 
 		err = fdt_overlay_apply_verbose(ctx->fdt, blob);
-		if (err) {
+		if (err)
 			printf("Failed to apply overlay %s, skipping\n",
 			       overlayfile);
-			goto skip_overlay;
-		}
-
-skip_overlay:
-		if (end)
-			free(overlayfile);
-	} while ((fdtoverlay = strstr(fdtoverlay, " ")));
+	}
 }
 
 const char *pxe_get_fdt_fallback(struct pxe_label *label, ulong kern_addr)
@@ -501,7 +475,7 @@  static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label)
 	if (label->kaslrseed)
 		label_boot_kaslrseed(ctx);
 
-	if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays)
+	if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays.count)
 		label_boot_fdtoverlay(ctx, label);
 
 	return 0;
diff --git a/include/pxe_utils.h b/include/pxe_utils.h
index ec7f12cd7bf..52a520e4823 100644
--- a/include/pxe_utils.h
+++ b/include/pxe_utils.h
@@ -39,7 +39,7 @@ 
  * @initrd: path to the initrd to use for this label.
  * @fdt: path to FDT to use
  * @fdtdir: path to FDT directory to use
- * @fdtoverlays: space-separated list of paths of FDT overlays to apply
+ * @fdtoverlays: list of paths of FDT overlays to apply (alist of char *)
  * @say: message to print when this label is selected for booting
  * @ipappend: flags for appending IP address (0x1) and MAC address (0x3)
  * @attempted: 0 if we haven't tried to boot this label, 1 if we have
@@ -59,7 +59,7 @@  struct pxe_label {
 	char *initrd;
 	char *fdt;
 	char *fdtdir;
-	char *fdtoverlays;
+	struct alist fdtoverlays;
 	char *say;
 	int ipappend;
 	int attempted;
diff --git a/test/boot/pxe.c b/test/boot/pxe.c
index 7132d318c56..47cfa4042f5 100644
--- a/test/boot/pxe.c
+++ b/test/boot/pxe.c
@@ -190,8 +190,11 @@  static int pxe_test_parse_norun(struct unit_test_state *uts)
 	ut_asserteq_str("/initrd.img", label->initrd);
 	ut_asserteq_str("/dtb/board.dtb", label->fdt);
 	ut_assertnull(label->fdtdir);
-	ut_asserteq_str("/dtb/overlay1.dtbo /dtb/overlay2.dtbo",
-			label->fdtoverlays);
+	ut_asserteq(2, label->fdtoverlays.count);
+	ut_asserteq_str("/dtb/overlay1.dtbo",
+			*alist_get(&label->fdtoverlays, 0, char *));
+	ut_asserteq_str("/dtb/overlay2.dtbo",
+			*alist_get(&label->fdtoverlays, 1, char *));
 	ut_asserteq_str("Booting default Linux kernel", label->say);
 	ut_asserteq(0, label->ipappend);
 	ut_asserteq(0, label->attempted);
@@ -211,7 +214,7 @@  static int pxe_test_parse_norun(struct unit_test_state *uts)
 	ut_assertnull(label->initrd);
 	ut_assertnull(label->fdt);
 	ut_asserteq_str("/dtb/", label->fdtdir);
-	ut_assertnull(label->fdtoverlays);
+	ut_asserteq(0, label->fdtoverlays.count);
 	ut_assertnull(label->say);
 	ut_asserteq(3, label->ipappend);
 	ut_asserteq(0, label->attempted);
@@ -231,7 +234,7 @@  static int pxe_test_parse_norun(struct unit_test_state *uts)
 	ut_assertnull(label->initrd);
 	ut_assertnull(label->fdt);
 	ut_assertnull(label->fdtdir);
-	ut_assertnull(label->fdtoverlays);
+	ut_asserteq(0, label->fdtoverlays.count);
 	ut_assertnull(label->say);
 	ut_asserteq(0, label->ipappend);
 	ut_asserteq(0, label->attempted);
@@ -251,7 +254,7 @@  static int pxe_test_parse_norun(struct unit_test_state *uts)
 	ut_assertnull(label->initrd);
 	ut_assertnull(label->fdt);
 	ut_assertnull(label->fdtdir);
-	ut_assertnull(label->fdtoverlays);
+	ut_asserteq(0, label->fdtoverlays.count);
 	ut_assertnull(label->say);
 	ut_asserteq(0, label->ipappend);
 	ut_asserteq(0, label->attempted);
@@ -271,7 +274,8 @@  static int pxe_test_parse_norun(struct unit_test_state *uts)
 	ut_assertnull(label->initrd);
 	ut_assertnull(label->fdt);
 	ut_assertnull(label->fdtdir);
-	ut_assertnull(label->fdtoverlays);
+	ut_asserteq(0, label->fdtoverlays.count);
+	ut_assertnull(label->say);
 	ut_asserteq(0, label->ipappend);
 	ut_asserteq(0, label->attempted);
 	ut_asserteq(0, label->localboot);
@@ -662,7 +666,7 @@  static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts)
 	/* Get the first label (linux) which has fdtoverlays */
 	label = list_first_entry(&cfg->labels, struct pxe_label, list);
 	ut_asserteq_str("linux", label->name);
-	ut_assertnonnull(label->fdtoverlays);
+	ut_assert(label->fdtoverlays.count > 0);
 
 	/* Enable output for loading phase */
 	ctx.quiet = false;