public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH 02/15] net: wget: let wget_with_dns work with dns disabled
@ 2024-11-08  3:45 Adriano Cordova
  2024-11-08  6:59 ` Heinrich Schuchardt
  0 siblings, 1 reply; 2+ messages in thread
From: Adriano Cordova @ 2024-11-08  3:45 UTC (permalink / raw)
  To: u-boot
  Cc: joe.hershberger, rfried.dev, jerome.forissier, xypron.glpk,
	ilias.apalodimas, Adriano Cordova

This was marked as TODO in the code:
-Enable use of wget_with_dns even if CMD_DNS is disabled if
the given uri has the ip address for the http server.
-Check for port in the uri when transforming to legacy wget
syntax inside wget_with_dns.
-Move the check for CMD_DNS inside wget_with_dns.

Signed-off-by: Adriano Cordova <adrianox@gmail.com>
---
 net/wget.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/net/wget.c b/net/wget.c
index 2d931c2bc6..5bb61a2b8d 100644
--- a/net/wget.c
+++ b/net/wget.c
@@ -528,12 +528,10 @@ void wget_start(void)
 	wget_send(TCP_SYN, 0, 0, 0);
 }
 
-#if (IS_ENABLED(CONFIG_CMD_DNS))
 int wget_with_dns(ulong dst_addr, char *uri)
 {
 	int ret;
-	char *s, *host_name, *file_name, *str_copy;
-
+	char *s, *host_name, *file_name, *str_copy, *port;
 	/*
 	 * Download file using wget.
 	 *
@@ -554,18 +552,31 @@ int wget_with_dns(ulong dst_addr, char *uri)
 	}
 	file_name = s;
 
-	/* TODO: If the given uri has ip address for the http server, skip dns */
-	net_dns_resolve = host_name;
-	net_dns_env_var = "httpserverip";
-	if (net_loop(DNS) < 0) {
-		log_err("Error: dns lookup of %s failed, check setup\n", net_dns_resolve);
-		ret = -EINVAL;
-		goto out;
-	}
-	s = env_get("httpserverip");
-	if (!s) {
+	port = host_name;
+	host_name = strsep(&port, ":");
+
+	if (string_to_ip(host_name).s_addr != 0) {
+		s = host_name;
+	} else {
+#if (IS_ENABLED(CONFIG_CMD_DNS))
+		net_dns_resolve = host_name;
+		net_dns_env_var = "httpserverip";
+		if (net_loop(DNS) < 0) {
+			log_err("Error: dns lookup of %s failed, check setup\n", net_dns_resolve);
+			ret = -EINVAL;
+			goto out;
+		}
+		s = env_get("httpserverip");
+		if (!s) {
+			log_err("Error:  %s could not be resolved\n", net_dns_resolve);
+			ret = -EINVAL;
+			goto out;
+		}
+#else
+		log_err("Error: dns disabled, %s could not be resolved\n", host_name);
 		ret = -EINVAL;
 		goto out;
+#endif
 	}
 
 	strlcpy(net_boot_file_name, s, sizeof(net_boot_file_name));
@@ -579,7 +590,6 @@ out:
 
 	return ret < 0 ? ret : 0;
 }
-#endif
 
 /**
  * wget_validate_uri() - validate the uri for wget
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH 02/15] net: wget: let wget_with_dns work with dns disabled
  2024-11-08  3:45 [PATCH 02/15] net: wget: let wget_with_dns work with dns disabled Adriano Cordova
@ 2024-11-08  6:59 ` Heinrich Schuchardt
  0 siblings, 0 replies; 2+ messages in thread
From: Heinrich Schuchardt @ 2024-11-08  6:59 UTC (permalink / raw)
  To: Adriano Cordova, u-boot
  Cc: joe.hershberger, rfried.dev, jerome.forissier, ilias.apalodimas

Am 8. November 2024 04:45:01 MEZ schrieb Adriano Cordova <adrianox@gmail.com>:
>This was marked as TODO in the code:
>-Enable use of wget_with_dns even if CMD_DNS is disabled if
>the given uri has the ip address for the http server.
>-Check for port in the uri when transforming to legacy wget
>syntax inside wget_with_dns.
>-Move the check for CMD_DNS inside wget_with_dns.
>
>Signed-off-by: Adriano Cordova <adrianox@gmail.com>
>---
> net/wget.c | 38 ++++++++++++++++++++++++--------------
> 1 file changed, 24 insertions(+), 14 deletions(-)
>
>diff --git a/net/wget.c b/net/wget.c
>index 2d931c2bc6..5bb61a2b8d 100644
>--- a/net/wget.c
>+++ b/net/wget.c
>@@ -528,12 +528,10 @@ void wget_start(void)
> 	wget_send(TCP_SYN, 0, 0, 0);
> }
> 
>-#if (IS_ENABLED(CONFIG_CMD_DNS))
> int wget_with_dns(ulong dst_addr, char *uri)
> {
> 	int ret;
>-	char *s, *host_name, *file_name, *str_copy;
>-
>+	char *s, *host_name, *file_name, *str_copy, *port;
> 	/*
> 	 * Download file using wget.
> 	 *
>@@ -554,18 +552,31 @@ int wget_with_dns(ulong dst_addr, char *uri)
> 	}
> 	file_name = s;
> 
>-	/* TODO: If the given uri has ip address for the http server, skip dns */
>-	net_dns_resolve = host_name;
>-	net_dns_env_var = "httpserverip";
>-	if (net_loop(DNS) < 0) {
>-		log_err("Error: dns lookup of %s failed, check setup\n", net_dns_resolve);
>-		ret = -EINVAL;
>-		goto out;
>-	}
>-	s = env_get("httpserverip");
>-	if (!s) {
>+	port = host_name;
>+	host_name = strsep(&port, ":");
>+
>+	if (string_to_ip(host_name).s_addr != 0) {
>+		s = host_name;
>+	} else {
>+#if (IS_ENABLED(CONFIG_CMD_DNS))

The U-Boot project prefers to use 'if' instead of #if' (if it compiles). This allows the compiler to check both paths for syntax and then eliminate the unused one. We are using the -O2 optimization setting.

If you run scripts/checkpatch.pl, you will see a warning.

>+		net_dns_resolve = host_name;
>+		net_dns_env_var = "httpserverip";
>+		if (net_loop(DNS) < 0) {
>+			log_err("Error: dns lookup of %s failed, check setup\n", net_dns_resolve);
>+			ret = -EINVAL;
>+			goto out;
>+		}
>+		s = env_get("httpserverip");
>+		if (!s) {
>+			log_err("Error:  %s could not be resolved\n", net_dns_resolve);
>+			ret = -EINVAL;
>+			goto out;
>+		}
>+#else
>+		log_err("Error: dns disabled, %s could not be resolved\n", host_name);

I would tend to remove "Error:" in all messages as it provides no additional information.

Best regards

Heinrich

> 		ret = -EINVAL;
> 		goto out;
>+#endif
> 	}
> 
> 	strlcpy(net_boot_file_name, s, sizeof(net_boot_file_name));
>@@ -579,7 +590,6 @@ out:
> 
> 	return ret < 0 ? ret : 0;
> }
>-#endif
> 
> /**
>  * wget_validate_uri() - validate the uri for wget


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-11-08 13:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-08  3:45 [PATCH 02/15] net: wget: let wget_with_dns work with dns disabled Adriano Cordova
2024-11-08  6:59 ` Heinrich Schuchardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox