All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Glass <sjg@chromium.org>
To: U-Boot Mailing List <u-boot@lists.denx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Steffen Jaeckel <jaeckel-floss@eyet-services.de>,
	Michal Simek <michal.simek@xilinx.com>,
	Tom Rini <trini@konsulko.com>, Dennis Gilmore <dennis@ausil.us>,
	Daniel Schwierzeck <daniel.schwierzeck@gmail.com>,
	Lukas Auer <lukas.auer@aisec.fraunhofer.de>,
	Simon Glass <sjg@chromium.org>
Subject: [PATCH 11/28] pxe: Clean up the use of bootfile
Date: Wed, 18 Aug 2021 21:45:44 -0600	[thread overview]
Message-ID: <20210818214547.11.I6a01e73ef114448e39cb6899c21f6c169e4da216@changeid> (raw)
In-Reply-To: <20210819034601.1618773-1-sjg@chromium.org>

The 'bootfile' environment variable is read in the bowels of pxe_util to
provide a directory to which all loaded files are relative.

This is not obvious from the API to PXE and it is strange to make the
caller set an environment variable rather than pass this as a parameter.

The code is also convoluted, which this feature implemented by
get_bootfile_path().

Update the API to improve this. Unfortunately this means that
pxe_setup_ctx() can fail, so add error checking.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/pxe_utils.c    | 60 ++++++++++++++++++++++++++++-----------------
 cmd/pxe.c           | 18 +++++++++++---
 cmd/sysboot.c       | 15 +++++++++---
 include/pxe_utils.h | 19 +++++++++++---
 4 files changed, 79 insertions(+), 33 deletions(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 225729ce57f..c04be110ea4 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -67,10 +67,10 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len)
 /**
  * get_bootfile_path() - Figure out the path of a file to read
  *
- * Returns the directory the file specified in the 'bootfile' env variable is
- * in. If bootfile isn't defined in the environment, return NULL, which should
- * be interpreted as "don't prepend anything to paths".
+ * Copies the boot directory into the supplied buffer. If there is no boot
+ * directory, set it to ""
  *
+ * @ctx: PXE context
  * @file_path: File path to read (relative to the PXE file)
  * @bootfile_path: Place to put the bootfile path
  * @bootfile_path_size: Size of @bootfile_path in bytes
@@ -79,34 +79,25 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len)
  * Returns 1 for success, -ENOSPC if bootfile_path_size is to small to hold the
  *	resulting path
  */
-static int get_bootfile_path(const char *file_path, char *bootfile_path,
-			     size_t bootfile_path_size, bool allow_abs_path)
+static int get_bootfile_path(struct pxe_context *ctx, const char *file_path,
+			     char *bootfile_path, size_t bootfile_path_size,
+			     bool allow_abs_path)
 {
-	char *bootfile, *last_slash;
 	size_t path_len = 0;
 
 	/* Only syslinux allows absolute paths */
 	if (file_path[0] == '/' && allow_abs_path)
 		goto ret;
 
-	bootfile = from_env("bootfile");
-	if (!bootfile)
-		goto ret;
-
-	last_slash = strrchr(bootfile, '/');
-	if (!last_slash)
-		goto ret;
-
-	path_len = (last_slash - bootfile) + 1;
-
-	if (bootfile_path_size < path_len) {
+	path_len = strlen(ctx->bootdir);
+	if (bootfile_path_size < path_len + 1) {
 		printf("bootfile_path too small. (%zd < %zd)\n",
 		       bootfile_path_size, path_len);
 
 		return -ENOSPC;
 	}
 
-	strncpy(bootfile_path, bootfile, path_len);
+	strncpy(bootfile_path, ctx->bootdir, path_len);
 
  ret:
 	bootfile_path[path_len] = '\0';
@@ -135,7 +126,7 @@ static int get_relfile(struct pxe_context *ctx, const char *file_path,
 	char addr_buf[18];
 	int err;
 
-	err = get_bootfile_path(file_path, relfile, sizeof(relfile),
+	err = get_bootfile_path(ctx, file_path, relfile, sizeof(relfile),
 				ctx->allow_abs_path);
 	if (err < 0)
 		return err;
@@ -1477,14 +1468,39 @@ void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg)
 	boot_unattempted_labels(ctx, cfg);
 }
 
-void pxe_setup_ctx(struct pxe_context *ctx, struct cmd_tbl *cmdtp,
-		   pxe_getfile_func getfile, void *userdata,
-		   bool allow_abs_path)
+int pxe_setup_ctx(struct pxe_context *ctx, struct cmd_tbl *cmdtp,
+		  pxe_getfile_func getfile, void *userdata,
+		  bool allow_abs_path, const char *bootfile)
 {
+	const char *last_slash;
+	size_t path_len = 0;
+
+	memset(ctx, '\0', sizeof(*ctx));
 	ctx->cmdtp = cmdtp;
 	ctx->getfile = getfile;
 	ctx->userdata = userdata;
 	ctx->allow_abs_path = allow_abs_path;
+
+	/* figure out the boot directory, if there is one */
+	if (bootfile && strlen(bootfile) >= MAX_TFTP_PATH_LEN)
+		return -ENOSPC;
+	ctx->bootdir = strdup(bootfile ? bootfile : "");
+	if (!ctx->bootdir)
+		return -ENOMEM;
+
+	if (bootfile) {
+		last_slash = strrchr(bootfile, '/');
+		if (last_slash)
+			path_len = (last_slash - bootfile) + 1;
+	}
+	ctx->bootdir[path_len] = '\0';
+
+	return 0;
+}
+
+void pxe_destroy_ctx(struct pxe_context *ctx)
+{
+	free(ctx->bootdir);
 }
 
 int pxe_process(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt)
diff --git a/cmd/pxe.c b/cmd/pxe.c
index 4fa51d2e053..e319db51ef5 100644
--- a/cmd/pxe.c
+++ b/cmd/pxe.c
@@ -121,8 +121,6 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	struct pxe_context ctx;
 	int err, i = 0;
 
-	pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false);
-
 	if (argc != 1)
 		return CMD_RET_USAGE;
 
@@ -136,6 +134,11 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	if (err < 0)
 		return 1;
 
+	if (pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false,
+			  env_get("bootfile"))) {
+		printf("Out of memory\n");
+		return CMD_RET_FAILURE;
+	}
 	/*
 	 * Keep trying paths until we successfully get a file we're looking
 	 * for.
@@ -144,6 +147,7 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	    pxe_mac_path(&ctx, pxefile_addr_r) > 0 ||
 	    pxe_ipaddr_paths(&ctx, pxefile_addr_r) > 0) {
 		printf("Config file found\n");
+		pxe_destroy_ctx(&ctx);
 
 		return 0;
 	}
@@ -152,12 +156,14 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		if (get_pxelinux_path(&ctx, pxe_default_paths[i],
 				      pxefile_addr_r) > 0) {
 			printf("Config file found\n");
+			pxe_destroy_ctx(&ctx);
 			return 0;
 		}
 		i++;
 	}
 
 	printf("Config file not found\n");
+	pxe_destroy_ctx(&ctx);
 
 	return 1;
 }
@@ -175,8 +181,6 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	struct pxe_context ctx;
 	int ret;
 
-	pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false);
-
 	if (argc == 1) {
 		pxefile_addr_str = from_env("pxefile_addr_r");
 		if (!pxefile_addr_str)
@@ -193,7 +197,13 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		return 1;
 	}
 
+	if (pxe_setup_ctx(&ctx, cmdtp, do_get_tftp, NULL, false,
+			  env_get("bootfile"))) {
+		printf("Out of memory\n");
+		return CMD_RET_FAILURE;
+	}
 	ret = pxe_process(&ctx, pxefile_addr_r, false);
+	pxe_destroy_ctx(&ctx);
 	if (ret)
 		return CMD_RET_FAILURE;
 
diff --git a/cmd/sysboot.c b/cmd/sysboot.c
index 7ee14df79e5..c45fed774d6 100644
--- a/cmd/sysboot.c
+++ b/cmd/sysboot.c
@@ -59,6 +59,7 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc,
 		      char *const argv[])
 {
 	unsigned long pxefile_addr_r;
+	pxe_getfile_func getfile;
 	struct pxe_context ctx;
 	char *pxefile_addr_str;
 	char *filename;
@@ -89,13 +90,12 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc,
 		env_set("bootfile", filename);
 	}
 
-	pxe_setup_ctx(&ctx, cmdtp, NULL, NULL, true);
 	if (strstr(argv[3], "ext2")) {
-		ctx.getfile = do_get_ext2;
+		getfile = do_get_ext2;
 	} else if (strstr(argv[3], "fat")) {
-		ctx.getfile = do_get_fat;
+		getfile = do_get_fat;
 	} else if (strstr(argv[3], "any")) {
-		ctx.getfile = do_get_any;
+		getfile = do_get_any;
 	} else {
 		printf("Invalid filesystem: %s\n", argv[3]);
 		return 1;
@@ -108,12 +108,19 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc,
 		return 1;
 	}
 
+	if (pxe_setup_ctx(&ctx, cmdtp, getfile, NULL, true, filename)) {
+		printf("Out of memory\n");
+		return CMD_RET_FAILURE;
+	}
+
 	if (get_pxe_file(&ctx, filename, pxefile_addr_r) < 0) {
 		printf("Error reading config file\n");
+		pxe_destroy_ctx(&ctx);
 		return 1;
 	}
 
 	ret = pxe_process(&ctx, pxefile_addr_r, prompt);
+	pxe_destroy_ctx(&ctx);
 	if (ret)
 		return CMD_RET_FAILURE;
 
diff --git a/include/pxe_utils.h b/include/pxe_utils.h
index 0cae0dabec3..543d0245c8a 100644
--- a/include/pxe_utils.h
+++ b/include/pxe_utils.h
@@ -86,6 +86,8 @@ typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path,
  * @getfile: Function called by PXE to read a file
  * @userdata: Data the caller requires for @getfile
  * @allow_abs_path: true to allow absolute paths
+ * @bootdir: Directory that files are loaded from ("" if no directory). This is
+ *	allocated
  */
 struct pxe_context {
 	struct cmd_tbl *cmdtp;
@@ -102,6 +104,7 @@ struct pxe_context {
 
 	void *userdata;
 	bool allow_abs_path;
+	char *bootdir;
 };
 
 /**
@@ -197,10 +200,20 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len);
  * @getfile: Function to call to read a file
  * @userdata: Data the caller requires for @getfile - stored in ctx->userdata
  * @allow_abs_path: true to allow absolute paths
+ * @bootfile: Bootfile whose directory loaded files are relative to, NULL if
+ *	none
+ * @return 0 if OK, -ENOMEM if out of memory
  */
-void pxe_setup_ctx(struct pxe_context *ctx, struct cmd_tbl *cmdtp,
-		   pxe_getfile_func getfile, void *userdata,
-		   bool allow_abs_path);
+int pxe_setup_ctx(struct pxe_context *ctx, struct cmd_tbl *cmdtp,
+		  pxe_getfile_func getfile, void *userdata,
+		  bool allow_abs_path, const char *bootfile);
+
+/**
+ * pxe_destroy_ctx() - Destroy a PXE context
+ *
+ * @ctx: Context to destroy
+ */
+void pxe_destroy_ctx(struct pxe_context *ctx);
 
 /**
  * pxe_process() - Process a PXE file through to boot
-- 
2.33.0.rc1.237.g0d66db33f3-goog


  parent reply	other threads:[~2021-08-19  3:48 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-19  3:45 [PATCH 00/28] Initial implementation of bootmethod/bootflow Simon Glass
2021-08-19  3:45 ` [PATCH 01/28] Create a new boot/ directory Simon Glass
2021-08-19  3:45 ` [PATCH 02/28] pxe: Move API comments to the header files Simon Glass
2021-08-19  3:45 ` [PATCH 03/28] pxe: Use a context pointer Simon Glass
2021-08-19  3:45 ` [PATCH 04/28] pxe: Move do_getfile() into the context Simon Glass
2021-08-19  3:45 ` [PATCH 05/28] pxe: Add a userdata field to " Simon Glass
2021-08-19  3:45 ` [PATCH 06/28] pxe: Tidy up the is_pxe global Simon Glass
2021-08-19  3:45 ` [PATCH 07/28] pxe: Move pxe_utils files Simon Glass
2021-08-19  3:45 ` [PATCH 08/28] pxe: Tidy up some comments in pxe_utils Simon Glass
2021-08-19  3:45 ` [PATCH 09/28] pxe: Tidy up code style a little " Simon Glass
2021-08-19  3:45 ` [PATCH 10/28] pxe: Move common parsing coding into pxe_util Simon Glass
2021-08-19  3:45 ` Simon Glass [this message]
2021-08-19  3:45 ` [PATCH 12/28] pxe: Drop get_bootfile_path() Simon Glass
2021-08-19  3:45 ` [PATCH 13/28] lib: Add tests for simple_itoa() Simon Glass
2021-08-19  3:45 ` [PATCH 14/28] lib: Add a function to convert a string to a hex value Simon Glass
2021-08-19  3:45 ` [PATCH 15/28] pxe: Return the file size from the getfile() function Simon Glass
2021-08-19  3:45 ` [PATCH 16/28] pxe: Refactor sysboot to have one helper Simon Glass
2021-08-19  3:45 ` [PATCH 17/28] doc: Move distro boot doc to rST Simon Glass
2021-08-19  3:45 ` [PATCH 18/28] pxe: Allow calling the pxe_get logic directly Simon Glass
2021-08-19  3:45 ` [PATCH 19/28] bootmethod: Add the uclass and core implementation Simon Glass
2021-08-19  3:45 ` [PATCH 20/28] bootmethod: Add an implementation of distro boot Simon Glass
2021-08-19  3:45 ` [PATCH 21/28] bootmethod: Add a command Simon Glass
2021-08-19  3:45 ` [PATCH 22/28] bootflow: " Simon Glass
2021-08-19  3:45 ` [PATCH 23/28] bootmethod: Add tests for bootmethod and bootflow Simon Glass
2021-08-19  3:45 ` [PATCH 24/28] bootmethod: doc: Add documentation Simon Glass
2021-08-19  3:45 ` [PATCH 25/28] mmc: Allow for children other than the block device Simon Glass
2021-08-19  3:45 ` [PATCH 26/28] mmc: Add a bootmethod Simon Glass
2021-08-19  3:46 ` [PATCH 27/28] ethernet: " Simon Glass
2021-08-19  3:46 ` [PATCH 28/28] RFC: rpi: Switch over to use bootflow Simon Glass
2021-08-19 13:59 ` [PATCH 00/28] Initial implementation of bootmethod/bootflow Tom Rini
2021-08-19 14:25   ` Simon Glass
2021-08-19 17:27     ` Tom Rini
2021-08-23 12:35       ` Ilias Apalodimas
2021-08-23 17:25         ` Simon Glass
2021-08-23 20:08           ` Tom Rini
2021-08-24  9:29             ` Ilias Apalodimas
2021-08-25 13:11               ` Simon Glass
2021-08-25 13:29                 ` Peter Robinson
2021-08-25 21:34                   ` Mark Kettenis
2021-08-25 21:58                     ` Tom Rini
2021-08-20  3:15     ` AKASHI Takahiro
2021-08-20 18:22       ` Simon Glass
2021-08-23  0:46         ` AKASHI Takahiro
2021-08-23 11:54 ` Mark Kettenis
2021-08-23 17:25   ` Simon Glass
2021-08-23 20:01     ` Tom Rini
2021-08-24 10:22       ` Mark Kettenis
2021-08-25 10:45         ` Emmanuel Vadot
2021-08-25 13:11           ` Simon Glass
2021-08-25 14:42             ` AKASHI Takahiro
2021-08-25 14:56               ` Tom Rini
2021-08-25 21:54                 ` Mark Kettenis
2021-08-25 22:06                   ` Tom Rini
2021-08-26  6:33                     ` AKASHI Takahiro
2021-08-26 13:03                       ` Tom Rini
2021-08-26 12:01                     ` Mark Kettenis
2021-08-26 13:00                       ` Tom Rini
2021-08-26 13:32                         ` Mark Kettenis
2021-08-26 13:50                           ` Ilias Apalodimas
2021-08-26 11:55                 ` Peter Robinson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210818214547.11.I6a01e73ef114448e39cb6899c21f6c169e4da216@changeid \
    --to=sjg@chromium.org \
    --cc=daniel.schwierzeck@gmail.com \
    --cc=dennis@ausil.us \
    --cc=ilias.apalodimas@linaro.org \
    --cc=jaeckel-floss@eyet-services.de \
    --cc=lukas.auer@aisec.fraunhofer.de \
    --cc=michal.simek@xilinx.com \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.