[Concept,13/32] boot: pxe: Defer processing of include files

Message ID 20260109231151.4056804-14-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>

Instead of loading and parsing included files inline during parsing,
store them in cfg->includes and process them after the initial parse
completes. This separates the parsing logic from file loading.

Add struct pxe_include to store the path, target menu pointer and
nesting level. The nesting level ensures the MAX_NEST_LEVEL (16) limit
is properly enforced for deeply nested includes.

Add pxe_parse_include() for callers who want manual control over include
processing. The parse_pxefile() function processes includes automatically
to maintain backwards compatibility with existing callers.

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

 boot/pxe_parse.c    | 38 ++++++++++-----------------
 boot/pxe_utils.c    | 62 +++++++++++++++++++++++++++++++++++++++++----
 include/pxe_utils.h | 58 +++++++++++++++++++++++++++++++++++++-----
 test/boot/pxe.c     | 19 +++++++++-----
 4 files changed, 135 insertions(+), 42 deletions(-)
  

Patch

diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c
index e3412166a63..72a672354b5 100644
--- a/boot/pxe_parse.c
+++ b/boot/pxe_parse.c
@@ -326,39 +326,31 @@  static int parse_integer(char **c, int *dst)
 }
 
 /*
- * Parse an include statement, and retrieve and parse the file it mentions.
+ * Parse an include statement and store the path for later loading.
  *
- * base should point to a location where it's safe to store the file, and
- * nest_level should indicate how many nested includes have occurred. For this
- * include, nest_level has already been incremented and doesn't need to be
- * incremented here.
+ * The include is added to cfg->includes. The caller is responsible for
+ * loading these files and calling pxe_parse_include() to parse them.
  */
-static int handle_include(struct pxe_context *ctx, char **c, unsigned long base,
-			  struct pxe_menu *cfg, int nest_level)
+static int handle_include(char **c, struct pxe_menu *cfg, int nest_level)
 {
-	char *include_path;
+	struct pxe_include inc;
 	char *s = *c;
 	int err;
-	char *buf;
-	int ret;
 
-	err = parse_sliteral(c, &include_path);
+	err = parse_sliteral(c, &inc.path);
 	if (err < 0) {
 		printf("Expected include path: %.*s\n", (int)(*c - s), s);
 		return err;
 	}
+	inc.cfg = cfg;
+	inc.nest_level = nest_level + 1;
 
-	err = get_pxe_file(ctx, include_path, base);
-	if (err < 0) {
-		printf("Couldn't retrieve %s\n", include_path);
-		return err;
+	if (!alist_add(&cfg->includes, inc)) {
+		free(inc.path);
+		return -ENOMEM;
 	}
 
-	buf = map_sysmem(base, 0);
-	ret = parse_pxefile_top(ctx, buf, base, cfg, nest_level);
-	unmap_sysmem(buf);
-
-	return ret;
+	return 1;
 }
 
 /*
@@ -385,7 +377,7 @@  static int parse_menu(struct pxe_context *ctx, char **c, struct pxe_menu *cfg,
 		err = parse_sliteral(c, &cfg->title);
 		break;
 	case T_INCLUDE:
-		err = handle_include(ctx, c, base, cfg, nest_level + 1);
+		err = handle_include(c, cfg, nest_level);
 		break;
 	case T_BACKGROUND:
 		err = parse_sliteral(c, &cfg->bmp);
@@ -635,9 +627,7 @@  int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base,
 			}
 			break;
 		case T_INCLUDE:
-			err = handle_include(ctx, &p,
-					     base + ALIGN(strlen(b), 4), cfg,
-					     nest_level + 1);
+			err = handle_include(&p, cfg, nest_level);
 			break;
 		case T_PROMPT:
 			err = parse_integer(&p, &cfg->prompt);
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index e0b7faddbd0..410f2d1eafe 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -910,12 +910,14 @@  struct pxe_menu *pxe_menu_init(void)
 
 	memset(cfg, '\0', sizeof(struct pxe_menu));
 	INIT_LIST_HEAD(&cfg->labels);
+	alist_init(&cfg->includes, sizeof(struct pxe_include), 0);
 
 	return cfg;
 }
 
 void pxe_menu_uninit(struct pxe_menu *cfg)
 {
+	struct pxe_include *inc;
 	struct list_head *pos, *n;
 	struct pxe_label *label;
 
@@ -929,6 +931,10 @@  void pxe_menu_uninit(struct pxe_menu *cfg)
 		label_destroy(label);
 	}
 
+	alist_for_each(inc, &cfg->includes)
+		free(inc->path);
+	alist_uninit(&cfg->includes);
+
 	free(cfg);
 }
 
@@ -944,6 +950,12 @@  struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg)
 
 	buf = map_sysmem(menucfg, 0);
 	r = parse_pxefile_top(ctx, buf, menucfg, cfg, 1);
+	unmap_sysmem(buf);
+
+	if (r < 0) {
+		pxe_menu_uninit(cfg);
+		return NULL;
+	}
 
 	if (ctx->use_fallback) {
 		if (cfg->fallback_label) {
@@ -954,13 +966,46 @@  struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg)
 		}
 	}
 
-	unmap_sysmem(buf);
-	if (r < 0) {
-		pxe_menu_uninit(cfg);
-		return NULL;
+	return cfg;
+}
+
+int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg,
+			 ulong base)
+{
+	struct pxe_include *inc;
+	char *buf;
+	uint i;
+	int r;
+
+	/*
+	 * Process includes - load each file and parse it. Get the include
+	 * fresh each iteration since parsing may add more includes and cause
+	 * alist reallocation.
+	 */
+	for (i = 0; i < cfg->includes.count; i++) {
+		inc = alist_getw(&cfg->includes, i, struct pxe_include);
+
+		r = get_pxe_file(ctx, inc->path, base);
+		if (r < 0) {
+			printf("Couldn't retrieve %s\n", inc->path);
+			return r;
+		}
+
+		buf = map_sysmem(base, 0);
+		r = pxe_parse_include(ctx, inc, buf, base);
+		unmap_sysmem(buf);
+
+		if (r < 0)
+			return r;
 	}
 
-	return cfg;
+	return 0;
+}
+
+int pxe_parse_include(struct pxe_context *ctx, struct pxe_include *inc,
+		      char *buf, ulong base)
+{
+	return parse_pxefile_top(ctx, buf, base, inc->cfg, inc->nest_level);
 }
 
 /*
@@ -1179,6 +1224,7 @@  struct pxe_menu *pxe_prepare(struct pxe_context *ctx, ulong pxefile_addr_r,
 			     bool prompt)
 {
 	struct pxe_menu *cfg;
+	int ret;
 
 	cfg = parse_pxefile(ctx, pxefile_addr_r);
 	if (!cfg) {
@@ -1186,6 +1232,12 @@  struct pxe_menu *pxe_prepare(struct pxe_context *ctx, ulong pxefile_addr_r,
 		return NULL;
 	}
 
+	ret = pxe_process_includes(ctx, cfg, pxefile_addr_r);
+	if (ret) {
+		pxe_menu_uninit(cfg);
+		return NULL;
+	}
+
 	if (prompt)
 		cfg->prompt = prompt;
 
diff --git a/include/pxe_utils.h b/include/pxe_utils.h
index 0ea250300d6..7f5b8c040d6 100644
--- a/include/pxe_utils.h
+++ b/include/pxe_utils.h
@@ -3,6 +3,7 @@ 
 #ifndef __PXE_UTILS_H
 #define __PXE_UTILS_H
 
+#include <alist.h>
 #include <bootflow.h>
 #include <linux/list.h>
 
@@ -68,6 +69,19 @@  struct pxe_label {
 	struct list_head list;
 };
 
+/**
+ * struct pxe_include - an include file that needs to be loaded
+ *
+ * @path: Path to the include file
+ * @cfg: Menu to parse the include into
+ * @nest_level: Nesting level to use when parsing this include
+ */
+struct pxe_include {
+	char *path;
+	struct pxe_menu *cfg;
+	int nest_level;
+};
+
 /*
  * Describes a pxe menu as given via pxe files.
  *
@@ -81,6 +95,7 @@  struct pxe_label {
  *          interrupted.  If 1, always prompt for a choice regardless of
  *          timeout.
  * labels - a list of labels defined for the menu.
+ * includes - list of struct pxe_include for files that need loading/parsing
  */
 struct pxe_menu {
 	char *title;
@@ -90,6 +105,7 @@  struct pxe_menu {
 	int timeout;
 	int prompt;
 	struct list_head labels;
+	struct alist includes;
 };
 
 struct pxe_context;
@@ -242,18 +258,48 @@  int get_pxelinux_path(struct pxe_context *ctx, const char *file,
 void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg);
 
 /**
- * parse_pxefile() - Parsing a pxe file
+ * parse_pxefile() - Parse a pxe file
  *
- * This is only used for the top-level file.
+ * Parse the top-level file. Any includes are stored in cfg->includes and
+ * should be processed by calling pxe_process_includes().
  *
  * @ctx: PXE context (provided by the caller)
- * Returns NULL if there is an error, otherwise, returns a pointer to a
- * pxe_menu struct populated with the results of parsing the pxe file (and any
- * files it includes). The resulting pxe_menu struct can be free()'d by using
- * the pxe_menu_uninit() function.
+ * @menucfg: Address of the PXE file in memory
+ * Return: NULL on error, otherwise a pointer to a pxe_menu struct. Use
+ * pxe_menu_uninit() to free it.
  */
 struct pxe_menu *parse_pxefile(struct pxe_context *ctx, ulong menucfg);
 
+/**
+ * pxe_process_includes() - Process include files in a parsed menu
+ *
+ * Load and parse all include files referenced in cfg->includes. This may
+ * add more includes if nested includes are found.
+ *
+ * @ctx: PXE context with getfile callback
+ * @cfg: Parsed PXE menu with includes to process
+ * @base: Memory address for loading include files
+ * Return: 0 on success, -ve on error
+ */
+int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg,
+			 ulong base);
+
+/**
+ * pxe_parse_include() - Parse an included file into its target menu
+ *
+ * After loading an include file referenced in cfg->includes, call this
+ * to parse it and merge any labels into the target menu. This may add
+ * more entries to cfg->includes if the included file has its own includes.
+ *
+ * @ctx: PXE context
+ * @inc: Include info with path and target menu
+ * @buf: Buffer containing the included file content
+ * @base: Memory address where buf is located
+ * Return: 1 on success, -ve on error
+ */
+int pxe_parse_include(struct pxe_context *ctx, struct pxe_include *inc,
+		      char *buf, ulong base);
+
 /**
  * format_mac_pxe() - Convert a MAC address to PXE format
  *
diff --git a/test/boot/pxe.c b/test/boot/pxe.c
index 8d7c6f7f2ae..365aff9f37a 100644
--- a/test/boot/pxe.c
+++ b/test/boot/pxe.c
@@ -164,6 +164,9 @@  static int pxe_test_parse_norun(struct unit_test_state *uts)
 	cfg = parse_pxefile(&ctx, addr);
 	ut_assertnonnull(cfg);
 
+	/* Process any include files */
+	ut_assertok(pxe_process_includes(&ctx, cfg, addr));
+
 	/* Verify no console output during parsing (say is printed on boot) */
 	ut_assert_console_end();
 
@@ -621,7 +624,6 @@  static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts)
 	struct pxe_menu *cfg;
 	ulong addr = PXE_LOAD_ADDR;
 	void *fdt;
-	uint i;
 
 	ut_assertnonnull(fs_image);
 	ut_assertnonnull(cfg_path);
@@ -635,17 +637,17 @@  static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts)
 	ut_assertok(pxe_setup_ctx(&ctx, pxe_test_getfile, &info, true, cfg_path,
 				  false, false, NULL));
 
-	/* Read and parse the config file */
+	/* Read and parse the config file (quiet since we're just parsing) */
+	ctx.quiet = true;
 	ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr));
 
 	cfg = parse_pxefile(&ctx, addr);
 	ut_assertnonnull(cfg);
 
-	/* Consume parsing output (say message is printed on boot, not parsing) */
-	ut_assert_nextline("Retrieving file: %s", cfg_path);
-	ut_assert_nextline("Retrieving file: /extlinux/extra.conf");
-	for (i = 3; i <= 16; i++)
-		ut_assert_nextline("Retrieving file: /extlinux/nest%d.conf", i);
+	/* Process any include files */
+	ut_assertok(pxe_process_includes(&ctx, cfg, addr));
+
+	/* Verify no console output during parsing (say is printed on boot) */
 	ut_assert_console_end();
 
 	/*
@@ -662,6 +664,9 @@  static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts)
 	ut_asserteq_str("linux", label->name);
 	ut_assertnonnull(label->fdtoverlays);
 
+	/* Enable output for loading phase */
+	ctx.quiet = false;
+
 	/* Load the label - should succeed but skip overlays */
 	ut_assertok(pxe_load_label(&ctx, label));