[systemd-devel] [PATCH 6/6] refactor in_addr_to_string()

[Ronny Chevalier]

2015-03-11 16:13 GMT+01:00 Shawn Landden <shawn at churchofgit.com>:
> ---

Hi,

>  src/resolve/resolved-dns-rr.c |  6 ++----
>  src/shared/in-addr-util.c     | 32 +++++++++++---------------------
>  2 files changed, 13 insertions(+), 25 deletions(-)
>
> diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c
> index 78d9e4a..a73ccd7 100644
> --- a/src/resolve/resolved-dns-rr.c
> +++ b/src/resolve/resolved-dns-rr.c
> @@ -527,13 +527,11 @@ int dns_resource_record_to_string(const DnsResourceRecord *rr, char **ret) {
>                  break;
>
>          case DNS_TYPE_A: {
> -                _cleanup_free_ char *x = NULL;
> -
> -                r = in_addr_to_string(AF_INET, (const union in_addr_union*) &rr->a.in_addr, &x);
> +                r = in_addr_to_string(AF_INET, (const union in_addr_union*) &rr->a.in_addr, &t);
>                  if (r < 0)
>                          return r;
>
> -                s = strjoin(k, " ", x, NULL);
> +                s = strjoin(k, " ", t, NULL);
>                  if (!s)
>                          return -ENOMEM;
>                  break;
> diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
> index b7532a6..f23b343 100644
> --- a/src/shared/in-addr-util.c
> +++ b/src/shared/in-addr-util.c
> @@ -22,6 +22,7 @@
>  #include <arpa/inet.h>
>
>  #include "in-addr-util.h"
> +#include "socket-util.h"
>
>  int in_addr_is_null(int family, const union in_addr_union *u) {
>          assert(u);
> @@ -179,31 +180,20 @@ int in_addr_prefix_next(int family, union in_addr_union *u, unsigned prefixlen)
>  }
>
>  int in_addr_to_string(int family, const union in_addr_union *u, char **ret) {
> -        char *x;
> -        size_t l;
> +        union sockaddr_union sa;
>
> -        assert(u);
> -        assert(ret);
> +        sa.sa.sa_family = family;
>
> -        if (family == AF_INET)
> -                l = INET_ADDRSTRLEN;
> -        else if (family == AF_INET6)
> -                l = INET6_ADDRSTRLEN;
> -        else
> -                return -EAFNOSUPPORT;
> +        if (sa.sa.sa_family == AF_INET) {
> +                sa.in.sin_addr = u->in;
>
> -        x = new(char, l);
> -        if (!x)
> -                return -ENOMEM;
> +                return sockaddr_pretty(&sa.sa, (socklen_t)sizeof(sa.in),  false, false, ret);
> +        } else if (family == AF_INET6) {
> +                sa.in6.sin6_addr = u->in6;
>
> -        errno = 0;
> -        if (!inet_ntop(family, u, x, l)) {
> -                free(x);
> -                return errno ? -errno : -EINVAL;
> -        }
> -
> -        *ret = x;
> -        return 0;
> +                return sockaddr_pretty(&sa.sa, (socklen_t)sizeof(sa.in6), false, false, ret);
> +        } else
> +                return -EAFNOSUPPORT;
>  }
>

I don't see the benefit of refactoring this way.

sockaddr_pretty internally uses inet_ntop for AF_INET6 according to
the parameters you gave it, and for AF_INET it will gave us exactly
the same output without using inet_ntop. So in the end it would be the
same output, but it will add some overhead from sockaddr_pretty.

Could you elaborate?


Ronny