On Tue, 6 Oct 2020 08:58:38 +0200
Ján Tomko <jtomko@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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;
