[libvirt PATCH 02/14] qemu: agent: split out qemuAgentGetInterfaceOneAddress

[Jonathon Jongsma]

On Tue,  6 Oct 2020 08:58:38 +0200
Ján Tomko <jtomko at redhat.com> wrote:

> A function that takes one entry from the "ip-addresses" array
> returned by "guest-network-get-interfaces" and converts it
> into virDomainIPAddress.
> 
> Signed-off-by: Ján Tomko <jtomko at redhat.com>
> ---
>  src/qemu/qemu_agent.c | 78
> +++++++++++++++++++++++++------------------ 1 file changed, 46
> insertions(+), 32 deletions(-)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 456f0b69e6..fc7b65de2a 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -2059,6 +2059,51 @@ qemuAgentGetFSInfo(qemuAgentPtr agent,
>      return ret;
>  }
>  
> +
> +static int
> +qemuAgentGetInterfaceOneAddress(virDomainIPAddressPtr ip_addr,
> +                                virJSONValuePtr ip_addr_obj,
> +                                const char *name)
> +{
> +    const char *type, *addr;
> +
> +    type = virJSONValueObjectGetString(ip_addr_obj,
> "ip-address-type");
> +    if (!type) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("qemu agent didn't provide
> 'ip-address-type'"
> +                         " field for interface '%s'"), name);
> +        return -1;
> +    } else if (STREQ(type, "ipv4")) {
> +        ip_addr->type = VIR_IP_ADDR_TYPE_IPV4;
> +    } else if (STREQ(type, "ipv6")) {
> +        ip_addr->type = VIR_IP_ADDR_TYPE_IPV6;
> +    } else {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("unknown ip address type '%s'"),
> +                       type);
> +        return -1;
> +    }
> +
> +    addr = virJSONValueObjectGetString(ip_addr_obj, "ip-address");
> +    if (!addr) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("qemu agent didn't provide 'ip-address'"
> +                         " field for interface '%s'"), name);
> +        return -1;
> +    }
> +    ip_addr->addr = g_strdup(addr);

(This comment is also true of the existing code, but somehow it feels a
bit more fragile when it's extracted out to its own function)

If the "prefix" check below fails, we will have allocated memory for
the address but will return a failure status. That string may leak if
the calling code does not increment iface->naddrs even on failure.
Perhaps we should only allocate the string if all sanity checks pass and
we are guaranteed to return success from this function?


> +
> +    if (virJSONValueObjectGetNumberUint(ip_addr_obj, "prefix",
> +                                        &ip_addr->prefix) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("malformed 'prefix' field"));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  /*
>   * qemuAgentGetInterfaces:
>   * @agent: agent object
> @@ -2176,7 +2221,6 @@ qemuAgentGetInterfaces(qemuAgentPtr agent,
>          addrs_count = iface->naddrs;
>  
>          for (j = 0; j < virJSONValueArraySize(ip_addr_arr); j++) {
> -            const char *type, *addr;
>              virJSONValuePtr ip_addr_obj =
> virJSONValueArrayGet(ip_addr_arr, j); virDomainIPAddressPtr ip_addr;
>  
> @@ -2192,38 +2236,8 @@ qemuAgentGetInterfaces(qemuAgentPtr agent,
>                  goto error;
>              }
>  
> -            type = virJSONValueObjectGetString(ip_addr_obj,
> "ip-address-type");
> -            if (!type) {
> -                virReportError(VIR_ERR_INTERNAL_ERROR,
> -                               _("qemu agent didn't provide
> 'ip-address-type'"
> -                                 " field for interface '%s'"), name);
> +            if (qemuAgentGetInterfaceOneAddress(ip_addr,
> ip_addr_obj, name) < 0) goto error;
> -            } else if (STREQ(type, "ipv4")) {
> -                ip_addr->type = VIR_IP_ADDR_TYPE_IPV4;
> -            } else if (STREQ(type, "ipv6")) {
> -                ip_addr->type = VIR_IP_ADDR_TYPE_IPV6;
> -            } else {
> -                virReportError(VIR_ERR_INTERNAL_ERROR,
> -                               _("unknown ip address type '%s'"),
> -                               type);
> -                goto error;
> -            }
> -
> -            addr = virJSONValueObjectGetString(ip_addr_obj,
> "ip-address");
> -            if (!addr) {
> -                virReportError(VIR_ERR_INTERNAL_ERROR,
> -                               _("qemu agent didn't provide
> 'ip-address'"
> -                                 " field for interface '%s'"), name);
> -                goto error;
> -            }
> -            ip_addr->addr = g_strdup(addr);
> -
> -            if (virJSONValueObjectGetNumberUint(ip_addr_obj,
> "prefix",
> -                                                &ip_addr->prefix) <
> 0) {
> -                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                               _("malformed 'prefix' field"));
> -                goto error;
> -            }
>          }
>  
>          iface->naddrs = addrs_count;