On 12/05/2026 23:48, David Lechner wrote:
> Split wget_do_request() into two functions to make error handling less
> error-prone.
>
> After a successful call to net_lwip_new_netif(), net_lwip_remove_netif()
> must always be called to leaks. This was missed in the HTTPS section of
> the code where we returned on error without cleaning up.
>
> Instead of adding more calls to net_lwip_remove_netif(), refactor the
> code into two functions. The outer function handles managing the netif
> lifecycle. The inner function no longer has to worry about cleaning up
> before returning on error.
>
> To keep things simple, the `path` local variable is removed during the
> refactoring. Instead, ctx.path is used directly everywhere.
>
> Fixes: 3c656c928bd7 ("net: lwip: add wget command")
> Signed-off-by: David Lechner <[email protected]>
> ---
> net/lwip/wget.c | 88
> ++++++++++++++++++++++++++++++---------------------------
> 1 file changed, 47 insertions(+), 41 deletions(-)
>
> diff --git a/net/lwip/wget.c b/net/lwip/wget.c
> index 502c0faebb2..f89a6580b41 100644
> --- a/net/lwip/wget.c
> +++ b/net/lwip/wget.c
> @@ -292,45 +292,17 @@ static err_t httpc_headers_done_cb(httpc_state_t
> *connection, void *arg, struct
> #if CONFIG_IS_ENABLED(WGET_CACERT)
> #endif
>
> -int wget_do_request(ulong dst_addr, char *uri)
> +static int wget_handle_request(struct wget_ctx *ctx, bool is_https,
> + struct udevice *udev, struct netif *netif)
> {
> #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;
> - bool is_https;
> -
> - ctx.daddr = dst_addr;
> - ctx.saved_daddr = dst_addr;
> - ctx.done = NOT_DONE;
> - ctx.size = 0;
> - ctx.prevsize = 0;
> - ctx.start_time = 0;
> - ctx.content_len = 0;
> - ctx.hash_count = 0;
> -
> - if (parse_url(uri, ctx.server_name, &ctx.port, &path, &is_https))
> - return CMD_RET_USAGE;
> -
> - if (net_lwip_eth_start() < 0)
> - return CMD_RET_FAILURE;
> -
> - if (!wget_info)
> - wget_info = &default_wget_info;
> -
> - udev = eth_get_dev();
> -
> - netif = net_lwip_new_netif(udev);
> - if (!netif)
> - return -1;
>
> /* if URL with hostname init dns */
> - if (!ipaddr_aton(ctx.server_name, NULL) && net_lwip_dns_init())
> + if (!ipaddr_aton(ctx->server_name, NULL) && net_lwip_dns_init())
> return CMD_RET_FAILURE;
>
> memset(&conn, 0, sizeof(conn));
> @@ -374,11 +346,10 @@ int wget_do_request(ulong dst_addr, char *uri)
> tls_allocator.alloc = &altcp_tls_alloc;
> tls_allocator.arg =
> altcp_tls_create_config_client(ca, ca_sz,
> - ctx.server_name);
> + ctx->server_name);
>
> if (!tls_allocator.arg) {
> log_err("error: Cannot create a TLS connection\n");
> - net_lwip_remove_netif(netif);
> return -1;
> }
>
> @@ -388,24 +359,20 @@ int wget_do_request(ulong dst_addr, char *uri)
>
> conn.result_fn = httpc_result_cb;
> conn.headers_done_fn = httpc_headers_done_cb;
> - ctx.path = path;
> - if (httpc_get_file_dns(ctx.server_name, ctx.port, path, &conn,
> httpc_recv_cb,
> - &ctx, &state)) {
> - net_lwip_remove_netif(netif);
> + if (httpc_get_file_dns(ctx->server_name, ctx->port, ctx->path, &conn,
> + httpc_recv_cb, ctx, &state)) {
> return CMD_RET_FAILURE;
> }
>
> errno = 0;
>
> - while (!ctx.done) {
> + while (!ctx->done) {
> net_lwip_rx(udev, netif);
> if (ctrlc())
> break;
> }
>
> - net_lwip_remove_netif(netif);
> -
> - if (ctx.done == SUCCESS)
> + if (ctx->done == SUCCESS)
> return 0;
>
> if (errno == EPERM && !wget_info->silent)
> @@ -414,6 +381,45 @@ int wget_do_request(ulong dst_addr, char *uri)
> return -1;
> }
>
> +int wget_do_request(ulong dst_addr, char *uri)
> +{
> + struct udevice *udev;
> + struct wget_ctx ctx;
> + struct netif *netif;
> + bool is_https;
> + int ret;
> +
> + ctx.daddr = dst_addr;
> + ctx.saved_daddr = dst_addr;
> + ctx.done = NOT_DONE;
> + ctx.size = 0;
> + ctx.prevsize = 0;
> + ctx.start_time = 0;
> + ctx.content_len = 0;
> + ctx.hash_count = 0;
> +
> + if (parse_url(uri, ctx.server_name, &ctx.port, &ctx.path, &is_https))
> + return CMD_RET_USAGE;
> +
> + if (net_lwip_eth_start() < 0)
> + return CMD_RET_FAILURE;
> +
> + if (!wget_info)
> + wget_info = &default_wget_info;
> +
> + udev = eth_get_dev();
> +
> + netif = net_lwip_new_netif(udev);
> + if (!netif)
> + return -1;
> +
> + ret = wget_handle_request(&ctx, is_https, udev, netif);
> +
> + net_lwip_remove_netif(netif);
> +
> + return ret;
> +}
> +
> /**
> * wget_validate_uri() - validate the uri for wget
> *
>
Reviewed-by: Jerome Forissier <[email protected]>
Thanks,
--
Jerome
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
