[Slirp] [PATCH] Remove alloca() call in get_dns_addr_resolv_conf()

[Marc-André Lureau]

On Fri, May 7, 2021 at 5:32 PM Philippe Mathieu-Daudé <philmd at redhat.com>
wrote:

> The ALLOCA(3) man-page mentions its "use is discouraged".
>
> For now get_dns_addr_resolv_conf() is called with pointer to
> a in_addr/in6_addr structure, and its size. Declare a union
> of these structures on the stack, able to hold both of them.
> This allows us to remove the alloca() call, keeping the buffer
> on the stack.
>
> Add an assertion in the unlikely case another inet address
> is handled by this function.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd at redhat.com>
>


---
> Sorry no pull-req, I don't have my 2FA token with me.
>

Made one for you and merged it:
https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/84


> ---
>  src/slirp.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/src/slirp.c b/src/slirp.c
> index ee4ce36..5d60cb5 100644
> --- a/src/slirp.c
> +++ b/src/slirp.c
> @@ -249,9 +249,13 @@ static int get_dns_addr_resolv_conf(int af, void
> *pdns_addr, void *cached_addr,
>      char buff2[257];
>      FILE *f;
>      int found = 0;
> -    void *tmp_addr = alloca(addrlen);
> +    union {
> +        struct in_addr dns_addr;
> +        struct in6_addr dns6_addr;
> +    } tmp_addr;
>      unsigned if_index;
>
> +    assert(sizeof(tmp_addr) >= addrlen);
>      f = fopen("/etc/resolv.conf", "r");
>      if (!f)
>          return -1;
> @@ -267,13 +271,13 @@ static int get_dns_addr_resolv_conf(int af, void
> *pdns_addr, void *cached_addr,
>                  if_index = 0;
>              }
>
> -            if (!inet_pton(af, buff2, tmp_addr)) {
> +            if (!inet_pton(af, buff2, &tmp_addr)) {
>                  continue;
>              }
>              /* If it's the first one, set it to dns_addr */
>              if (!found) {
> -                memcpy(pdns_addr, tmp_addr, addrlen);
> -                memcpy(cached_addr, tmp_addr, addrlen);
> +                memcpy(pdns_addr, &tmp_addr, addrlen);
> +                memcpy(cached_addr, &tmp_addr, addrlen);
>                  if (scope_id) {
>                      *scope_id = if_index;
>                  }
> @@ -285,7 +289,7 @@ static int get_dns_addr_resolv_conf(int af, void
> *pdns_addr, void *cached_addr,
>                  break;
>              } else if (slirp_debug & DBG_MISC) {
>                  char s[INET6_ADDRSTRLEN];
> -                const char *res = inet_ntop(af, tmp_addr, s, sizeof(s));
> +                const char *res = inet_ntop(af, &tmp_addr, s, sizeof(s));
>                  if (!res) {
>                      res = "  (string conversion error)";
>                  }
> --
> 2.26.3
>
> _______________________________________________
> Slirp mailing list
> Slirp at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/slirp
>


-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/slirp/attachments/20210507/28f351b6/attachment.htm>