* [PATCH 1/1] cmd: fix tftpput command
@ 2022-09-03 12:21 Heinrich Schuchardt
2022-09-03 16:55 ` Simon Glass
0 siblings, 1 reply; 2+ messages in thread
From: Heinrich Schuchardt @ 2022-09-03 12:21 UTC (permalink / raw)
To: Joe Hershberger, Ramon Fried; +Cc: Simon Glass, u-boot, Heinrich Schuchardt
Calling tftpput with less than 2 arguments must lead to a failure.
If tftpput is called with two arguments, these are the address and
the size of the file to be transferred.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
cmd/net.c | 50 +++++++++++++++++++++++++++++++++++++-------------
1 file changed, 37 insertions(+), 13 deletions(-)
diff --git a/cmd/net.c b/cmd/net.c
index 3619c843d8..e1be7b431a 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -189,6 +189,19 @@ static void netboot_update_env(void)
#endif
}
+/**
+ * parse_addr_size() - parse address and size arguments
+ */
+static int parse_addr_size(char * const argv[])
+{
+ if (strict_strtoul(argv[1], 16, &image_save_addr) < 0 ||
+ strict_strtoul(argv[2], 16, &image_save_size) < 0) {
+ printf("Invalid address/size\n");
+ return CMD_RET_USAGE;
+ }
+ return 0;
+}
+
static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
char *const argv[])
{
@@ -199,6 +212,7 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
ulong addr;
net_boot_file_name_explicit = false;
+ *net_boot_file_name = '\0';
/* pre-set image_load_addr */
s = env_get("loadaddr");
@@ -207,12 +221,18 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
switch (argc) {
case 1:
+ if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT)
+ goto err_usage;
+
/* refresh bootfile name from env */
copy_filename(net_boot_file_name, env_get("bootfile"),
sizeof(net_boot_file_name));
break;
- case 2: /*
+ case 2:
+ if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT)
+ goto err_usage;
+ /*
* Only one arg - accept two forms:
* Just load address, or just boot file name. The latter
* form must be written in a format which can not be
@@ -232,28 +252,28 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
break;
case 3:
- image_load_addr = hextoul(argv[1], NULL);
- net_boot_file_name_explicit = true;
- copy_filename(net_boot_file_name, argv[2],
- sizeof(net_boot_file_name));
-
+ if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT) {
+ if (parse_addr_size(argv))
+ goto err_usage;
+ } else {
+ image_load_addr = hextoul(argv[1], NULL);
+ net_boot_file_name_explicit = true;
+ copy_filename(net_boot_file_name, argv[2],
+ sizeof(net_boot_file_name));
+ }
break;
#ifdef CONFIG_CMD_TFTPPUT
case 4:
- if (strict_strtoul(argv[1], 16, &image_save_addr) < 0 ||
- strict_strtoul(argv[2], 16, &image_save_size) < 0) {
- printf("Invalid address/size\n");
- return CMD_RET_USAGE;
- }
+ if (parse_addr_size(argv))
+ goto err_usage;
net_boot_file_name_explicit = true;
copy_filename(net_boot_file_name, argv[3],
sizeof(net_boot_file_name));
break;
#endif
default:
- bootstage_error(BOOTSTAGE_ID_NET_START);
- return CMD_RET_USAGE;
+ goto err_usage;
}
bootstage_mark(BOOTSTAGE_ID_NET_START);
@@ -282,6 +302,10 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
else
bootstage_error(BOOTSTAGE_ID_NET_DONE_ERR);
return rcode;
+
+err_usage:
+ bootstage_error(BOOTSTAGE_ID_NET_START);
+ return CMD_RET_USAGE;
}
#if defined(CONFIG_CMD_PING)
--
2.30.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 1/1] cmd: fix tftpput command
2022-09-03 12:21 [PATCH 1/1] cmd: fix tftpput command Heinrich Schuchardt
@ 2022-09-03 16:55 ` Simon Glass
0 siblings, 0 replies; 2+ messages in thread
From: Simon Glass @ 2022-09-03 16:55 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: Joe Hershberger, Ramon Fried, U-Boot Mailing List
Hi Heinrich,
On Sat, 3 Sept 2022 at 06:21, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Calling tftpput with less than 2 arguments must lead to a failure.
>
> If tftpput is called with two arguments, these are the address and
> the size of the file to be transferred.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> cmd/net.c | 50 +++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 37 insertions(+), 13 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
>
> diff --git a/cmd/net.c b/cmd/net.c
> index 3619c843d8..e1be7b431a 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -189,6 +189,19 @@ static void netboot_update_env(void)
> #endif
> }
>
> +/**
> + * parse_addr_size() - parse address and size arguments
> + */
> +static int parse_addr_size(char * const argv[])
> +{
> + if (strict_strtoul(argv[1], 16, &image_save_addr) < 0 ||
> + strict_strtoul(argv[2], 16, &image_save_size) < 0) {
> + printf("Invalid address/size\n");
> + return CMD_RET_USAGE;
> + }
> + return 0;
> +}
> +
> static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
> char *const argv[])
> {
> @@ -199,6 +212,7 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
> ulong addr;
>
> net_boot_file_name_explicit = false;
> + *net_boot_file_name = '\0';
>
> /* pre-set image_load_addr */
> s = env_get("loadaddr");
> @@ -207,12 +221,18 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
>
> switch (argc) {
> case 1:
> + if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT)
> + goto err_usage;
> +
> /* refresh bootfile name from env */
> copy_filename(net_boot_file_name, env_get("bootfile"),
> sizeof(net_boot_file_name));
> break;
>
> - case 2: /*
> + case 2:
> + if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT)
> + goto err_usage;
> + /*
> * Only one arg - accept two forms:
> * Just load address, or just boot file name. The latter
> * form must be written in a format which can not be
> @@ -232,28 +252,28 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
> break;
>
> case 3:
> - image_load_addr = hextoul(argv[1], NULL);
> - net_boot_file_name_explicit = true;
> - copy_filename(net_boot_file_name, argv[2],
> - sizeof(net_boot_file_name));
> -
> + if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT) {
> + if (parse_addr_size(argv))
> + goto err_usage;
> + } else {
> + image_load_addr = hextoul(argv[1], NULL);
> + net_boot_file_name_explicit = true;
> + copy_filename(net_boot_file_name, argv[2],
> + sizeof(net_boot_file_name));
> + }
> break;
>
> #ifdef CONFIG_CMD_TFTPPUT
> case 4:
> - if (strict_strtoul(argv[1], 16, &image_save_addr) < 0 ||
> - strict_strtoul(argv[2], 16, &image_save_size) < 0) {
> - printf("Invalid address/size\n");
> - return CMD_RET_USAGE;
> - }
> + if (parse_addr_size(argv))
> + goto err_usage;
> net_boot_file_name_explicit = true;
> copy_filename(net_boot_file_name, argv[3],
> sizeof(net_boot_file_name));
> break;
> #endif
> default:
> - bootstage_error(BOOTSTAGE_ID_NET_START);
> - return CMD_RET_USAGE;
> + goto err_usage;
> }
> bootstage_mark(BOOTSTAGE_ID_NET_START);
>
> @@ -282,6 +302,10 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc,
> else
> bootstage_error(BOOTSTAGE_ID_NET_DONE_ERR);
> return rcode;
> +
> +err_usage:
> + bootstage_error(BOOTSTAGE_ID_NET_START);
> + return CMD_RET_USAGE;
> }
>
> #if defined(CONFIG_CMD_PING)
> --
> 2.30.2
>
To me this would be better if the arg parsing all moved to a separate
function which returns an error code. It would avoid the goto.
Also perhaps this should be in the same series as the tftpput docs?
Regards,
Simon
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-09-03 16:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-03 12:21 [PATCH 1/1] cmd: fix tftpput command Heinrich Schuchardt
2022-09-03 16:55 ` Simon Glass
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.