On Thu, 10 Apr 2025 at 20:46, Jerome Forissier
<jerome.foriss...@linaro.org> wrote:
>
> Functions called from EFI applications should not call printf().
> Refactor wget_do_request() to implement this requirement.
>
> Signed-off-by: Jerome Forissier <jerome.foriss...@linaro.org>
> Reported-by: Heinrich Schuchardt <xypron.g...@gmx.de>
> ---
>
> Changes in v3:
> - New patch: net-lwip: wget_do_request(): do not print anything to the
> console
>
> net/lwip/wget.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/net/lwip/wget.c b/net/lwip/wget.c
> index ec098148835..d3628cf72de 100644
> --- a/net/lwip/wget.c
> +++ b/net/lwip/wget.c
> @@ -36,6 +36,7 @@ struct wget_ctx {
> ulong prevsize;
> ulong start_time;
> enum done_state done;
> + bool silent;
> };
>
> static void wget_lwip_fill_info(struct pbuf *hdr, u16_t hdr_len, u32_t
> hdr_cont_len)
> @@ -217,7 +218,8 @@ static err_t httpc_recv_cb(void *arg, struct altcp_pcb
> *pcb, struct pbuf *pbuf,
> memcpy((void *)ctx->daddr, buf->payload, buf->len);
> ctx->daddr += buf->len;
> ctx->size += buf->len;
> - if (ctx->size - ctx->prevsize > PROGRESS_PRINT_STEP_BYTES) {
> + if (!ctx->silent &&
> + ctx->size - ctx->prevsize > PROGRESS_PRINT_STEP_BYTES) {
> printf("#");
> ctx->prevsize = ctx->size;
> }
> @@ -255,11 +257,15 @@ static void httpc_result_cb(void *arg, httpc_result_t
> httpc_result,
> elapsed = get_timer(ctx->start_time);
> if (!elapsed)
> elapsed = 1;
> - if (rx_content_len > PROGRESS_PRINT_STEP_BYTES)
> - printf("\n");
> - printf("%u bytes transferred in %lu ms (", rx_content_len, elapsed);
> - print_size(rx_content_len / elapsed * 1000, "/s)\n");
> - printf("Bytes transferred = %lu (%lx hex)\n", ctx->size, ctx->size);
> + if (!ctx->silent) {
> + if (rx_content_len > PROGRESS_PRINT_STEP_BYTES)
> + printf("\n");
> + printf("%u bytes transferred in %lu ms (", rx_content_len,
> + elapsed);
> + print_size(rx_content_len / elapsed * 1000, "/s)\n");
> + printf("Bytes transferred = %lu (%lx hex)\n", ctx->size,
> + ctx->size);
> + }
> if (wget_info->set_bootdev)
> efi_set_bootdev("Http", ctx->server_name, ctx->path,
> map_sysmem(ctx->saved_daddr, 0),
> rx_content_len);
> @@ -372,13 +378,14 @@ static int set_cacert(char * const saddr, char * const
> ssz)
> #endif
> #endif /* CONFIG_WGET_CACERT || CONFIG_WGET_BUILTIN_CACERT */
>
> -static int wget_loop(struct udevice *udev, ulong dst_addr, char *uri)
> +static int wget_loop(ulong dst_addr, char *uri, bool silent)
Can we expand the scope to include the non-lwip functions as well? The
wget_request() API can then be modified to include the bool, and the
prints can be made silent only when calling from the EFI API -- we can
still get the error message when calling from within U-Boot. This
version does not print what the issue is, but just errors out, which
can be a bit confusing. I do see that the non-lwip wget.c module does
not work with a context. Maybe the bool can instead be in the
wget_http_info struct so that it can be used for both lwip and
non-lwip cases.
-sughosh
> {
> #if CONFIG_IS_ENABLED(WGET_HTTPS)
> altcp_allocator_t tls_allocator;
> #endif
> httpc_connection_t conn;
> httpc_state_t *state;
> + struct udevice *udev;
> struct netif *netif;
> struct wget_ctx ctx;
> char *path;
> @@ -390,6 +397,13 @@ static int wget_loop(struct udevice *udev, ulong
> dst_addr, char *uri)
> ctx.size = 0;
> ctx.prevsize = 0;
> ctx.start_time = 0;
> + ctx.silent = silent;
> +
> + if (!wget_info)
> + wget_info = &default_wget_info;
> +
> + net_lwip_set_current();
> + udev = eth_get_dev();
>
> if (parse_url(uri, ctx.server_name, &ctx.port, &path, &is_https))
> return CMD_RET_USAGE;
> @@ -471,12 +485,7 @@ static int wget_loop(struct udevice *udev, ulong
> dst_addr, char *uri)
>
> int wget_do_request(ulong dst_addr, char *uri)
> {
> - net_lwip_set_current();
> -
> - if (!wget_info)
> - wget_info = &default_wget_info;
> -
> - return wget_loop(eth_get_dev(), dst_addr, uri);
> + return wget_loop(dst_addr, uri, true);
> }
>
> int do_wget(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
> @@ -521,7 +530,7 @@ int do_wget(struct cmd_tbl *cmdtp, int flag, int argc,
> char * const argv[])
> return CMD_RET_FAILURE;
>
> wget_info = &default_wget_info;
> - if (wget_do_request(dst_addr, nurl))
> + if (wget_loop(dst_addr, nurl, false))
> return CMD_RET_FAILURE;
>
> return CMD_RET_SUCCESS;
> --
> 2.43.0
>
