[PATCH v3 1/3] net-lwip: wget_do_request(): do not print anything to the console

[Sughosh Ganu]

On Thu, 10 Apr 2025 at 20:46, Jerome Forissier
<jerome.forissier at 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.forissier at linaro.org>
> Reported-by: Heinrich Schuchardt <xypron.glpk at 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
>