[libvirt] [PATCH 06/11] Expand virSocketFormat to be more flexible

[Eric Blake]

On 10/21/2010 12:17 PM, Daniel P. Berrange wrote:
> The getnameinfo() function is more flexible than inet_ntop()
> avoiding the need to if/else the code based on socket family.
> Also make it support UNIX socket addrs and allow inclusion
> of a port (service) address. Finally do proper error reporting
> via normal APIs.
>
> * src/conf/domain_conf.c, src/nwfilter/nwfilter_ebiptables_driver.c,
>    src/qemu/qemu_conf.c: Fix error handling with virSocketFormat
> * src/util/network.c: Rewrite virSocketFormat to use getnameinfo
>    and cope with UNIX socket addrs.
> ---
>   po/POTFILES.in                            |    1 +
>   src/conf/domain_conf.c                    |    5 +--
>   src/libvirt_private.syms                  |    1 +
>   src/nwfilter/nwfilter_ebiptables_driver.c |   12 +----
>   src/qemu/qemu_conf.c                      |    2 +
>   src/util/network.c                        |   82 ++++++++++++++++++++++-------
>   src/util/network.h                        |    5 ++
>   7 files changed, 75 insertions(+), 33 deletions(-)

> +/*
> + * virSocketFormatAddr:

s/Addr:/AddrFull:/

> + * @addr: an initialized virSocketAddrPtr
> + * @withService: if true, then service info is appended
> + * @separator: separator between hostname&  service.

Must be non-NULL if withService is true, and ignored when withService is 
false?  In which case, is it merely sufficient to provide a non-NULL 
string to request withService, in which point the second parameter is 
redundant?

> + *
> + * Returns a string representation of the given address
> + * Returns NULL on any error
> + * Caller must free the returned string
> + */
> +char *
> +virSocketFormatAddrFull(virSocketAddrPtr addr,
> +                        bool withService,
> +                        const char *separator)
> +{
> +    char host[NI_MAXHOST], port[NI_MAXSERV];
> +    char *addrstr;
> +    int err;
>
> -    else if (addr->data.stor.ss_family == AF_INET6) {
> -        outlen = INET6_ADDRSTRLEN;
> -        inaddr =&addr->data.inet6.sin6_addr;
> +    if (addr == NULL) {
> +        virSocketError(VIR_ERR_INVALID_ARG, _("Missing address"));
> +        return NULL;
>       }
>
> -    else {
> -        return NULL;
> +    /* Short-circuit since getnameinfo doesn't work
> +     * nicely for UNIX sockets */
> +    if (addr->data.sa.sa_family == AF_UNIX) {
> +        if (withService) {
> +            if (virAsprintf(&addrstr, "127.0.0.1%s0",
> +                            separator ? separator : ":")<  0)

Oh, I see - separator defaults to ":" if withService is true but 
separator is NULL.  Still, is the three-argument version buying us 
anything over a two-argument version where the caller must supply ":" 
rather than relying on a default?

ACK if you decide 3 arguments is worth it, and with the doc typo fixed.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org