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

[Masahisa Kojima]

Hi Ilias,

On Thu, 14 Sept 2023 at 21:52, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> 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

Yes.
Note that the prototype of this function is the same as the  Maxim's LWIP port
"int ulwip_wget(ulong addr, char *url)", so we can easily replace it
when the 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
OK.

>
> > +     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.

strlcat() appends the string with the limit of "buf_size - strlen(dst)
- 1", so current code should be OK.
1024 should be sizeof(net_boot_file_name) same as above.

Thanks,
Masahisa Kojima

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