Re: [PATCH v2 2/6] net: wget: add wget with dns utility function

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Kojima-san

[...]

>  }
> +
> +#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;
> +
> +	/*
> +	 * Download file using wget.
> +	 *
> +	 * U-Boot wget takes the target uri in this format.
> +	 *  "<http server ip>:<file path>"  e.g.) 192.168.1.1:/sample/test.iso
> +	 * Need to resolve the http server ip address before starting wget.
> +	 */
> +	str_copy = strdup(uri);
> +	if (!str_copy)
> +		return -ENOMEM;
> +
> +	s = str_copy + strlen("http://");
> +	host_name = strsep(&s, "/");
> +	if (!s) {
> +		log_err("Error: invalied uri, no file path\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	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";

This is not idea, but I understand this is how dns currently works.  We
can take another look on improving this when LWIP lands

> +	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) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	strlcpy(net_boot_file_name, s, 1024);

sizeof(net_boot_file_name) here please

> +	strlcat(net_boot_file_name, ":/", 1024); /* append '/' which is removed by strsep() */
> +	strlcat(net_boot_file_name, file_name, 1024);

Don't you have to limit the size of subsequent writes depending on what
previous calls wrote? IOW that can't always be 1024.

> +	image_load_addr = dst_addr;
> +	ret = net_loop(WGET);
> +
> +out:
> +	free(str_copy);
> +
> +	return ret;
> +}
> +#endif
> --
> 2.34.1
>

Thanks
/Ilias