Re: [PATCH v3 15/18] pxe: Return the file size from the getfile() function
On Thu, Oct 14, 2021 at 12:48:08PM -0600, Simon Glass wrote: > It is pretty strange that the pxe code uses the 'filesize' environment > variable find the size of a file it has just read. > > Partly this is because it uses the command-line interpreter to parse its > request to load the file. > > As a first step towards unwinding this, return it directly from the > getfile() function. This makes the code a bit longer, for now, but will be > cleaned up in future patches. > > Signed-off-by: Simon Glass > Reviewed-by: Artem Lapkin > Tested-by: Artem Lapkin > Reviewed-by: Ramon Fried Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 15/18] pxe: Return the file size from the getfile() function
On Thu, Oct 14, 2021 at 9:51 PM Simon Glass wrote:
>
> It is pretty strange that the pxe code uses the 'filesize' environment
> variable find the size of a file it has just read.
>
> Partly this is because it uses the command-line interpreter to parse its
> request to load the file.
>
> As a first step towards unwinding this, return it directly from the
> getfile() function. This makes the code a bit longer, for now, but will be
> cleaned up in future patches.
>
> Signed-off-by: Simon Glass
> ---
>
> (no changes since v1)
>
> boot/pxe_utils.c| 70 -
> cmd/pxe.c | 7 -
> cmd/sysboot.c | 21 --
> include/pxe_utils.h | 13 -
> 4 files changed, 79 insertions(+), 32 deletions(-)
>
> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> index f36f1f8a60c..e377e16be56 100644
> --- a/boot/pxe_utils.c
> +++ b/boot/pxe_utils.c
> @@ -30,6 +30,20 @@
>
> #define MAX_TFTP_PATH_LEN 512
>
> +int pxe_get_file_size(ulong *sizep)
> +{
> + const char *val;
> +
> + val = from_env("filesize");
> + if (!val)
> + return -ENOENT;
> +
> + if (strict_strtoul(val, 16, sizep) < 0)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> /**
> * format_mac_pxe() - obtain a MAC address in the PXE format
> *
> @@ -75,14 +89,17 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len)
> * @ctx: PXE context
> * @file_path: File path to read (relative to the PXE file)
> * @file_addr: Address to load file to
> + * @filesizep: If not NULL, returns the file size in bytes
> * Returns 1 for success, or < 0 on error
> */
> static int get_relfile(struct pxe_context *ctx, const char *file_path,
> - unsigned long file_addr)
> + unsigned long file_addr, ulong *filesizep)
> {
> size_t path_len;
> char relfile[MAX_TFTP_PATH_LEN + 1];
> char addr_buf[18];
> + ulong size;
> + int ret;
>
> if (file_path[0] == '/' && ctx->allow_abs_path)
> *relfile = '\0';
> @@ -103,7 +120,13 @@ static int get_relfile(struct pxe_context *ctx, const
> char *file_path,
>
> sprintf(addr_buf, "%lx", file_addr);
>
> - return ctx->getfile(ctx, relfile, addr_buf);
> + ret = ctx->getfile(ctx, relfile, addr_buf, &size);
> + if (ret < 0)
> + return log_msg_ret("get", ret);
> + if (filesizep)
> + *filesizep = size;
> +
> + return 1;
> }
>
> /**
> @@ -117,29 +140,17 @@ static int get_relfile(struct pxe_context *ctx, const
> char *file_path,
> * Returns 1 for success, or < 0 on error
> */
> int get_pxe_file(struct pxe_context *ctx, const char *file_path,
> -unsigned long file_addr)
> +ulong file_addr)
> {
> - unsigned long config_file_size;
> - char *tftp_filesize;
> + ulong size;
> int err;
> char *buf;
>
> - err = get_relfile(ctx, file_path, file_addr);
> + err = get_relfile(ctx, file_path, file_addr, &size);
> if (err < 0)
> return err;
>
> - /*
> -* the file comes without a NUL byte at the end, so find out its size
> -* and add the NUL byte.
> -*/
> - tftp_filesize = from_env("filesize");
> - if (!tftp_filesize)
> - return -ENOENT;
> -
> - if (strict_strtoul(tftp_filesize, 16, &config_file_size) < 0)
> - return -EINVAL;
> -
> - buf = map_sysmem(file_addr + config_file_size, 1);
> + buf = map_sysmem(file_addr + size, 1);
> *buf = '\0';
> unmap_sysmem(buf);
>
> @@ -184,12 +195,13 @@ int get_pxelinux_path(struct pxe_context *ctx, const
> char *file,
> * @file_path: File path to read (relative to the PXE file)
> * @envaddr_name: Name of environment variable which contains the address to
> * load to
> + * @filesizep: Returns the file size in bytes
> * Returns 1 on success, -ENOENT if @envaddr_name does not exist as an
> * environment variable, -EINVAL if its format is not valid hex, or other
> * value < 0 on other error
> */
> static int get_relfile_envaddr(struct pxe_context *ctx, const char
> *file_path,
> - const char *envaddr_name)
> + const char *envaddr_name, ulong *filesizep)
> {
> unsigned long file_addr;
> char *envaddr;
> @@ -201,7 +213,7 @@ static int get_relfile_envaddr(struct pxe_context *ctx,
> const char *file_path,
> if (strict_strtoul(envaddr, 16, &file_addr) < 0)
> return -EINVAL;
>
> - return get_relfile(ctx, file_path, file_addr);
> + return get_relfile(ctx, file_path, file_addr, filesizep);
> }
>
> /**
> @@ -357,8 +369,8 @@ static void label_boot_fdtoverlay(struct pxe_context *ctx,
> goto skip_overlay;
>
> /* Load overlay file */
> - err = get_r
Re: [PATCH v3 15/18] pxe: Return the file size from the getfile() function
OK will be usefull Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST Reviewed-by: Artem Lapkin Tested-by: Artem Lapkin
[PATCH v3 15/18] pxe: Return the file size from the getfile() function
It is pretty strange that the pxe code uses the 'filesize' environment
variable find the size of a file it has just read.
Partly this is because it uses the command-line interpreter to parse its
request to load the file.
As a first step towards unwinding this, return it directly from the
getfile() function. This makes the code a bit longer, for now, but will be
cleaned up in future patches.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/pxe_utils.c| 70 -
cmd/pxe.c | 7 -
cmd/sysboot.c | 21 --
include/pxe_utils.h | 13 -
4 files changed, 79 insertions(+), 32 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index f36f1f8a60c..e377e16be56 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -30,6 +30,20 @@
#define MAX_TFTP_PATH_LEN 512
+int pxe_get_file_size(ulong *sizep)
+{
+ const char *val;
+
+ val = from_env("filesize");
+ if (!val)
+ return -ENOENT;
+
+ if (strict_strtoul(val, 16, sizep) < 0)
+ return -EINVAL;
+
+ return 0;
+}
+
/**
* format_mac_pxe() - obtain a MAC address in the PXE format
*
@@ -75,14 +89,17 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len)
* @ctx: PXE context
* @file_path: File path to read (relative to the PXE file)
* @file_addr: Address to load file to
+ * @filesizep: If not NULL, returns the file size in bytes
* Returns 1 for success, or < 0 on error
*/
static int get_relfile(struct pxe_context *ctx, const char *file_path,
- unsigned long file_addr)
+ unsigned long file_addr, ulong *filesizep)
{
size_t path_len;
char relfile[MAX_TFTP_PATH_LEN + 1];
char addr_buf[18];
+ ulong size;
+ int ret;
if (file_path[0] == '/' && ctx->allow_abs_path)
*relfile = '\0';
@@ -103,7 +120,13 @@ static int get_relfile(struct pxe_context *ctx, const char
*file_path,
sprintf(addr_buf, "%lx", file_addr);
- return ctx->getfile(ctx, relfile, addr_buf);
+ ret = ctx->getfile(ctx, relfile, addr_buf, &size);
+ if (ret < 0)
+ return log_msg_ret("get", ret);
+ if (filesizep)
+ *filesizep = size;
+
+ return 1;
}
/**
@@ -117,29 +140,17 @@ static int get_relfile(struct pxe_context *ctx, const
char *file_path,
* Returns 1 for success, or < 0 on error
*/
int get_pxe_file(struct pxe_context *ctx, const char *file_path,
-unsigned long file_addr)
+ulong file_addr)
{
- unsigned long config_file_size;
- char *tftp_filesize;
+ ulong size;
int err;
char *buf;
- err = get_relfile(ctx, file_path, file_addr);
+ err = get_relfile(ctx, file_path, file_addr, &size);
if (err < 0)
return err;
- /*
-* the file comes without a NUL byte at the end, so find out its size
-* and add the NUL byte.
-*/
- tftp_filesize = from_env("filesize");
- if (!tftp_filesize)
- return -ENOENT;
-
- if (strict_strtoul(tftp_filesize, 16, &config_file_size) < 0)
- return -EINVAL;
-
- buf = map_sysmem(file_addr + config_file_size, 1);
+ buf = map_sysmem(file_addr + size, 1);
*buf = '\0';
unmap_sysmem(buf);
@@ -184,12 +195,13 @@ int get_pxelinux_path(struct pxe_context *ctx, const char
*file,
* @file_path: File path to read (relative to the PXE file)
* @envaddr_name: Name of environment variable which contains the address to
* load to
+ * @filesizep: Returns the file size in bytes
* Returns 1 on success, -ENOENT if @envaddr_name does not exist as an
* environment variable, -EINVAL if its format is not valid hex, or other
* value < 0 on other error
*/
static int get_relfile_envaddr(struct pxe_context *ctx, const char *file_path,
- const char *envaddr_name)
+ const char *envaddr_name, ulong *filesizep)
{
unsigned long file_addr;
char *envaddr;
@@ -201,7 +213,7 @@ static int get_relfile_envaddr(struct pxe_context *ctx,
const char *file_path,
if (strict_strtoul(envaddr, 16, &file_addr) < 0)
return -EINVAL;
- return get_relfile(ctx, file_path, file_addr);
+ return get_relfile(ctx, file_path, file_addr, filesizep);
}
/**
@@ -357,8 +369,8 @@ static void label_boot_fdtoverlay(struct pxe_context *ctx,
goto skip_overlay;
/* Load overlay file */
- err = get_relfile_envaddr(ctx, overlayfile,
- "fdtoverlay_addr_r");
+ err = get_relfile_envaddr(ctx, overlayfile, "fdtoverlay_addr_r",
+ NULL);
if (err < 0) {
printf("Failed loading overlay %s\n", o
