| Message ID | 20250410151600.3370247-2-jerome.forissier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | NET_LWIP LMB fixes | expand |
On Thu, 10 Apr 2025 at 20:46, Jerome Forissier <jerome.forissier@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@linaro.org> > Reported-by: Heinrich Schuchardt <xypron.glpk@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 >
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) { #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;
Functions called from EFI applications should not call printf(). Refactor wget_do_request() to implement this requirement. Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> Reported-by: Heinrich Schuchardt <xypron.glpk@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(-)