[PATCH v4 2/4] net, net-lwip: wget: suppress console output when called by EFI

Jerome Forissier jerome.forissier at linaro.org
Wed Apr 16 13:14:52 CEST 2025


Hi Sughosh,

On 4/16/25 12:22, Sughosh Ganu wrote:
> On Wed, 16 Apr 2025 at 01:04, Jerome Forissier
> <jerome.forissier at linaro.org> wrote:
>>
>> Functions called from EFI applications should not do console output.
>> Refactor the wget code to implement this requirement. The wget_http_info
>> struct is used to hold the boolean that signifies whether the output is
>> allowed or not.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier at linaro.org>
>> Reported-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>
>> Changes in v4:
>> - Patch renamed, deals with NET in addition to NET_LWIP
>>
>> Changes in v3:
>> - New patch
>>
>>  include/net-common.h     |  2 ++
>>  lib/efi_loader/efi_net.c |  2 +-
>>  net/lwip/wget.c          | 33 +++++++++++++++++++++++----------
>>  net/wget.c               | 23 +++++++++++++++++------
>>  4 files changed, 43 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/net-common.h b/include/net-common.h
>> index 30860f5975a..1043b24d0b9 100644
>> --- a/include/net-common.h
>> +++ b/include/net-common.h
>> @@ -555,6 +555,7 @@ enum wget_http_method {
>>   *                     Filled by client.
>>   * @hdr_cont_len:      content length according to headers. Filled by wget
>>   * @headers:           buffer for headers. Filled by wget.
>> + * @silent:            do not print anything to the console. Filled by client.
>>   */
>>  struct wget_http_info {
>>         enum wget_http_method method;
>> @@ -565,6 +566,7 @@ struct wget_http_info {
>>         bool check_buffer_size;
>>         u32 hdr_cont_len;
>>         char *headers;
>> +       bool silent;
>>  };
>>
>>  extern struct wget_http_info default_wget_info;
>> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
>> index b3291b4f1d5..9ff0b691ee1 100644
>> --- a/lib/efi_loader/efi_net.c
>> +++ b/lib/efi_loader/efi_net.c
>> @@ -51,7 +51,7 @@ static int next_dp_entry;
>>  static struct wget_http_info efi_wget_info = {
>>         .set_bootdev = false,
>>         .check_buffer_size = true,
>> -
>> +       .silent = true,
>>  };
>>  #endif
>>
>> diff --git a/net/lwip/wget.c b/net/lwip/wget.c
>> index 2b512a1bc84..3529768eb06 100644
>> --- a/net/lwip/wget.c
>> +++ b/net/lwip/wget.c
>> @@ -217,7 +217,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 (!wget_info->silent &&
>> +                   ctx->size - ctx->prevsize > PROGRESS_PRINT_STEP_BYTES) {
>>                         printf("#");
>>                         ctx->prevsize = ctx->size;
>>                 }
>> @@ -255,11 +256,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 (!wget_info->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);
>> @@ -339,7 +344,8 @@ static int _set_cacert(const void *addr, size_t sz)
>>         mbedtls_x509_crt_init(&crt);
>>         ret = mbedtls_x509_crt_parse(&crt, cacert, cacert_size);
>>         if (ret) {
>> -               printf("Could not parse certificates (%d)\n", ret);
>> +               if (!wget_info->silent)
>> +                       printf("Could not parse certificates (%d)\n", ret);
>>                 free(cacert);
>>                 cacert = NULL;
>>                 cacert_size = 0;
>> @@ -392,6 +398,12 @@ int wget_do_request(ulong dst_addr, char *uri)
>>         ctx.prevsize = 0;
>>         ctx.start_time = 0;
>>
>> +       if (!wget_info)
>> +               wget_info = &default_wget_info;
>> +
>> +       net_lwip_set_current();
>> +       udev = eth_get_dev();
>> +
> 
> Wasn't this already done in the first patch? Looks like this hunk is
> superfluous. Apart from that, it looks fine to me. Thanks for taking
> up this change.

Indeed, I messed up when rebasing. I'll remove this hunk.

Thank you,
-- 
Jerome

> 
> -sughosh
> 
>>         if (parse_url(uri, ctx.server_name, &ctx.port, &path, &is_https))
>>                 return CMD_RET_USAGE;
>>
>> @@ -421,9 +433,10 @@ int wget_do_request(ulong dst_addr, char *uri)
>>
>>                 if (cacert_auth_mode == AUTH_REQUIRED) {
>>                         if (!ca || !ca_sz) {
>> -                               printf("Error: cacert authentication mode is "
>> -                                      "'required' but no CA certificates "
>> -                                      "given\n");
>> +                               if (!wget_info->silent)
>> +                                       printf("Error: cacert authentication "
>> +                                              "mode is 'required' but no CA "
>> +                                              "certificates given\n");
>>                                 return CMD_RET_FAILURE;
>>                        }
>>                 } else if (cacert_auth_mode == AUTH_NONE) {
>> diff --git a/net/wget.c b/net/wget.c
>> index c73836cbc9d..3c0fff488eb 100644
>> --- a/net/wget.c
>> +++ b/net/wget.c
>> @@ -59,8 +59,10 @@ static inline int store_block(uchar *src, unsigned int offset, unsigned int len)
>>         if (CONFIG_IS_ENABLED(LMB) && wget_info->set_bootdev) {
>>                 if (store_addr < image_load_addr ||
>>                     lmb_read_check(store_addr, len)) {
>> -                       printf("\nwget error: ");
>> -                       printf("trying to overwrite reserved memory...\n");
>> +                       if (!wget_info->silent) {
>> +                               printf("\nwget error: ");
>> +                               printf("trying to overwrite reserved memory\n");
>> +                       }
>>                         return -1;
>>                 }
>>         }
>> @@ -76,6 +78,9 @@ static void show_block_marker(u32 packets)
>>  {
>>         int cnt;
>>
>> +       if (wget_info->silent)
>> +               return;
>> +
>>         if (content_length != -1) {
>>                 if (net_boot_file_size > content_length)
>>                         content_length = net_boot_file_size;
>> @@ -101,11 +106,15 @@ static void tcp_stream_on_closed(struct tcp_stream *tcp)
>>         net_set_state(wget_loop_state);
>>         if (wget_loop_state != NETLOOP_SUCCESS) {
>>                 net_boot_file_size = 0;
>> -               printf("\nwget: Transfer Fail, TCP status - %d\n", tcp->status);
>> +               if (!wget_info->silent)
>> +                       printf("\nwget: Transfer Fail, TCP status - %d\n",
>> +                              tcp->status);
>>                 return;
>>         }
>>
>> -       printf("\nPackets received %d, Transfer Successful\n", tcp->rx_packets);
>> +       if (!wget_info->silent)
>> +               printf("\nPackets received %d, Transfer Successful\n",
>> +                      tcp->rx_packets);
>>         wget_info->file_size = net_boot_file_size;
>>         if (wget_info->method == WGET_HTTP_METHOD_GET && wget_info->set_bootdev) {
>>                 efi_set_bootdev("Http", NULL, image_url,
>> @@ -139,7 +148,8 @@ static void tcp_stream_on_rcv_nxt_update(struct tcp_stream *tcp, u32 rx_bytes)
>>                     tcp->state == TCP_ESTABLISHED)
>>                         goto end;
>>
>> -               printf("ERROR: misssed HTTP header\n");
>> +               if (!wget_info->silent)
>> +                       printf("ERROR: misssed HTTP header\n");
>>                 tcp_stream_close(tcp);
>>                 goto end;
>>         }
>> @@ -346,7 +356,8 @@ void wget_start(void)
>>         tcp_stream_set_on_create_handler(tcp_stream_on_create);
>>         tcp = tcp_stream_connect(web_server_ip, server_port);
>>         if (!tcp) {
>> -               printf("No free tcp streams\n");
>> +               if (!wget_info->silent)
>> +                       printf("No free tcp streams\n");
>>                 net_set_state(NETLOOP_FAIL);
>>                 return;
>>         }
>> --
>> 2.43.0
>>


More information about the U-Boot mailing list