virJSONValueObjectGetArray returns NULL if the object with the supplied key is not an array. Calling virJSONValueIsArray right after is redundant. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 09116e749c..456f0b69e6 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1444,12 +1444,6 @@ qemuAgentGetVCPUs(qemuAgentPtr agent, goto cleanup; } - if (!virJSONValueIsArray(data)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Malformed guest-get-vcpus data array")); - goto cleanup; - } - ndata = virJSONValueArraySize(data); *info = g_new0(qemuAgentCPUInfo, ndata); @@ -2314,12 +2308,6 @@ qemuAgentGetUsers(qemuAgentPtr agent, return -1; } - if (!virJSONValueIsArray(data)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Malformed guest-get-users data array")); - return -1; - } - ndata = virJSONValueArraySize(data); if (virTypedParamsAddUInt(params, nparams, maxparams, -- 2.26.2

For both 'ip_addr_arr' and 'ret_array', we: 1) already checked that they are arrays 2) only iterate up to the array size Remove the duplicate checks. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index fc7b65de2a..51c597f57e 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2160,13 +2160,6 @@ qemuAgentGetInterfaces(qemuAgentPtr agent, const char *hwaddr, *ifname_s, *name = NULL; virDomainInterfacePtr iface = NULL; - /* Shouldn't happen but doesn't hurt to check neither */ - if (!tmp_iface) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu agent reply missing interface entry in array")); - goto error; - } - /* interface name is required to be presented */ name = virJSONValueObjectGetString(tmp_iface, "name"); if (!name) { @@ -2229,13 +2222,6 @@ qemuAgentGetInterfaces(qemuAgentPtr agent, ip_addr = &iface->addrs[addrs_count - 1]; - /* Shouldn't happen but doesn't hurt to check neither */ - if (!ip_addr_obj) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu agent reply missing IP addr in array")); - goto error; - } - if (qemuAgentGetInterfaceOneAddress(ip_addr, ip_addr_obj, name) < 0) goto error; } -- 2.26.2

qemuAgentGetInterfaceOneAddress returns exactly one address for every iteration of the loop (and we error out if not). Instead of expanding the addrs by one on every iteration, do it upfront since we know how many times the loop will execute. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index c6878c8590..0394a72518 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2213,20 +2213,18 @@ qemuAgentGetInterfaces(qemuAgentPtr agent, /* If current iface already exists, continue with the count */ addrs_count = iface->naddrs; + if (VIR_EXPAND_N(iface->addrs, addrs_count, + virJSONValueArraySize(ip_addr_arr)) < 0) + goto error; + for (j = 0; j < virJSONValueArraySize(ip_addr_arr); j++) { virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j); - virDomainIPAddressPtr ip_addr; - - if (VIR_EXPAND_N(iface->addrs, addrs_count, 1) < 0) - goto error; - - ip_addr = &iface->addrs[addrs_count - 1]; + virDomainIPAddressPtr ip_addr = iface->addrs + iface->naddrs; + iface->naddrs++; if (qemuAgentGetInterfaceOneAddress(ip_addr, ip_addr_obj, name) < 0) goto error; } - - iface->naddrs = addrs_count; } *ifaces = g_steal_pointer(&ifaces_ret); -- 2.26.2

We're passing 'ifaces_count' to virHashCreate as the initial hash table size just after we've initialized it to zero. This translates to a default of 256 inside virHashCreateFull. Instead of this obfuscation, use virHashNew (default of 32), to make it obvious we don't care about the initial hash size. Also remove the error handling, since neither of the functions return any errors after switching to g_new0. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index b11f8afde7..a09fb4da2a 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2129,10 +2129,7 @@ qemuAgentGetInterfaces(qemuAgentPtr agent, virHashTablePtr ifaces_store = NULL; /* Hash table to handle the interface alias */ - if (!(ifaces_store = virHashCreate(ifaces_count, NULL))) { - virHashFree(ifaces_store); - return -1; - } + ifaces_store = virHashNew(NULL); if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL))) goto cleanup; -- 2.26.2

Convert one interface from the "return" array returned by "guest-network-get-interfaces" to virDomainInterface. Due to the functionality of squashing interface aliases together, this is not a pure function - it either: * Adds the interface to ifaces_ret, incrementing ifaces_count and adds a pointer to it into the ifaces_store hash table. * Adds the additional IP addresses from the interface alias to the existing interface entry, found through the hash table. This does not increment ifaces_count or extend the array. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 162 +++++++++++++++++++++++++----------------- 1 file changed, 98 insertions(+), 64 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 721ff55fff..e614fdd7c4 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2104,6 +2104,101 @@ qemuAgentGetInterfaceOneAddress(virDomainIPAddressPtr ip_addr, } +/** + * qemuAgentGetInterfaceAddresses: + * @ifaces_ret: the array to put/update the interface in + * @ifaces_count: the number of interfaces in that array + * @ifaces_store: hash table into @ifaces_ret by interface name + * @tmp_iface: one item from the JSON array of interfaces + * + * This function processes @tmp_iface (which represents + * information about a single interface) and adds the information + * into the ifaces_ret array. + * + * If we're processing an interface alias, the suffix is stripped + * and information is appended to the entry found via the @ifaces_store + * hash table. + * + * Otherwise, the next free position in @ifaces_ret is used, + * its address added to @ifaces_store, and @ifaces_count incremented. + */ +static int +qemuAgentGetInterfaceAddresses(virDomainInterfacePtr **ifaces_ret, + size_t *ifaces_count, + virHashTablePtr ifaces_store, + virJSONValuePtr tmp_iface) +{ + virJSONValuePtr ip_addr_arr = NULL; + const char *hwaddr, *ifname_s, *name = NULL; + virDomainInterfacePtr iface = NULL; + g_auto(GStrv) ifname = NULL; + size_t addrs_count = 0; + size_t j; + + /* interface name is required to be presented */ + name = virJSONValueObjectGetString(tmp_iface, "name"); + if (!name) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't provide 'name' field")); + return -1; + } + + /* Handle interface alias (<ifname>:<alias>) */ + ifname = virStringSplit(name, ":", 2); + ifname_s = ifname[0]; + + iface = virHashLookup(ifaces_store, ifname_s); + + /* If the hash table doesn't contain this iface, add it */ + if (!iface) { + if (VIR_EXPAND_N(*ifaces_ret, *ifaces_count, 1) < 0) + return -1; + + iface = g_new0(virDomainInterface, 1); + (*ifaces_ret)[*ifaces_count - 1] = iface; + + if (virHashAddEntry(ifaces_store, ifname_s, iface) < 0) + return -1; + + iface->naddrs = 0; + iface->name = g_strdup(ifname_s); + + hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); + iface->hwaddr = g_strdup(hwaddr); + } + + /* as well as IP address which - moreover - + * can be presented multiple times */ + ip_addr_arr = virJSONValueObjectGet(tmp_iface, "ip-addresses"); + if (!ip_addr_arr) + return 0; + + if (!virJSONValueIsArray(ip_addr_arr)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed ip-addresses array")); + return -1; + } + + /* If current iface already exists, continue with the count */ + addrs_count = iface->naddrs; + + if (VIR_EXPAND_N(iface->addrs, addrs_count, + virJSONValueArraySize(ip_addr_arr)) < 0) + return -1; + + for (j = 0; j < virJSONValueArraySize(ip_addr_arr); j++) { + virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j); + virDomainIPAddressPtr ip_addr = iface->addrs + iface->naddrs; + iface->naddrs++; + + if (qemuAgentGetInterfaceOneAddress(ip_addr, ip_addr_obj, name) < 0) + return -1; + } + + return 0; +} + + /* * qemuAgentGetInterfaces: * @agent: agent object @@ -2120,7 +2215,7 @@ qemuAgentGetInterfaces(qemuAgentPtr agent, virDomainInterfacePtr **ifaces) { int ret = -1; - size_t i, j; + size_t i; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr ret_array = NULL; @@ -2151,71 +2246,10 @@ qemuAgentGetInterfaces(qemuAgentPtr agent, for (i = 0; i < virJSONValueArraySize(ret_array); i++) { virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i); - virJSONValuePtr ip_addr_arr = NULL; - const char *hwaddr, *ifname_s, *name = NULL; - virDomainInterfacePtr iface = NULL; - g_auto(GStrv) ifname = NULL; - size_t addrs_count = 0; - /* interface name is required to be presented */ - name = virJSONValueObjectGetString(tmp_iface, "name"); - if (!name) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu agent didn't provide 'name' field")); + if (qemuAgentGetInterfaceAddresses(&ifaces_ret, &ifaces_count, + ifaces_store, tmp_iface) < 0) goto error; - } - - /* Handle interface alias (<ifname>:<alias>) */ - ifname = virStringSplit(name, ":", 2); - ifname_s = ifname[0]; - - iface = virHashLookup(ifaces_store, ifname_s); - - /* If the hash table doesn't contain this iface, add it */ - if (!iface) { - if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0) - goto error; - - iface = g_new0(virDomainInterface, 1); - ifaces_ret[ifaces_count - 1] = iface; - - if (virHashAddEntry(ifaces_store, ifname_s, iface) < 0) - goto error; - - iface->naddrs = 0; - iface->name = g_strdup(ifname_s); - - hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); - iface->hwaddr = g_strdup(hwaddr); - } - - /* as well as IP address which - moreover - - * can be presented multiple times */ - ip_addr_arr = virJSONValueObjectGet(tmp_iface, "ip-addresses"); - if (!ip_addr_arr) - continue; - - if (!virJSONValueIsArray(ip_addr_arr)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Malformed ip-addresses array")); - goto error; - } - - /* If current iface already exists, continue with the count */ - addrs_count = iface->naddrs; - - if (VIR_EXPAND_N(iface->addrs, addrs_count, - virJSONValueArraySize(ip_addr_arr)) < 0) - goto error; - - for (j = 0; j < virJSONValueArraySize(ip_addr_arr); j++) { - virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j); - virDomainIPAddressPtr ip_addr = iface->addrs + iface->naddrs; - iface->naddrs++; - - if (qemuAgentGetInterfaceOneAddress(ip_addr, ip_addr_obj, name) < 0) - goto error; - } } *ifaces = g_steal_pointer(&ifaces_ret); -- 2.26.2

Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index e7ea03f840..39c955fdf1 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2216,12 +2216,12 @@ qemuAgentGetInterfaces(qemuAgentPtr agent, { int ret = -1; size_t i; - virJSONValuePtr cmd = NULL; - virJSONValuePtr reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; virJSONValuePtr ret_array = NULL; size_t ifaces_count = 0; virDomainInterfacePtr *ifaces_ret = NULL; - virHashTablePtr ifaces_store = NULL; + g_autoptr(virHashTable) ifaces_store = NULL; /* Hash table to handle the interface alias */ ifaces_store = virHashNew(NULL); @@ -2250,9 +2250,6 @@ qemuAgentGetInterfaces(qemuAgentPtr agent, ret = ifaces_count; cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - virHashFree(ifaces_store); return ret; error: -- 2.26.2

We only care about the first part of the 'ifname' string, splitting it is overkill. Instead, just replace the ':' with a '\0' in a copy of the string. This reduces the count of the varaibles containing some form of the interface name to two. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 06e143b0b9..5cb6257bfc 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2129,9 +2129,9 @@ qemuAgentGetInterfaceAddresses(virDomainInterfacePtr **ifaces_ret, virJSONValuePtr tmp_iface) { virJSONValuePtr ip_addr_arr = NULL; - const char *hwaddr, *ifname_s, *name = NULL; + const char *hwaddr, *name = NULL; virDomainInterfacePtr iface = NULL; - g_auto(GStrv) ifname = NULL; + g_autofree char *ifname = NULL; size_t addrs_count = 0; size_t j; @@ -2144,10 +2144,9 @@ qemuAgentGetInterfaceAddresses(virDomainInterfacePtr **ifaces_ret, } /* Handle interface alias (<ifname>:<alias>) */ - ifname = virStringSplit(name, ":", 2); - ifname_s = ifname[0]; + ifname = g_strdelimit(g_strdup(name), ":", '\0'); - iface = virHashLookup(ifaces_store, ifname_s); + iface = virHashLookup(ifaces_store, ifname); /* If the hash table doesn't contain this iface, add it */ if (!iface) { @@ -2157,11 +2156,11 @@ qemuAgentGetInterfaceAddresses(virDomainInterfacePtr **ifaces_ret, iface = g_new0(virDomainInterface, 1); (*ifaces_ret)[*ifaces_count - 1] = iface; - if (virHashAddEntry(ifaces_store, ifname_s, iface) < 0) + if (virHashAddEntry(ifaces_store, ifname, iface) < 0) return -1; iface->naddrs = 0; - iface->name = g_strdup(ifname_s); + iface->name = g_strdup(ifname); hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); iface->hwaddr = g_strdup(hwaddr); -- 2.26.2

For readability, and to ensure we do allocation when returning 0. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 301707b136..60247e1616 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2073,11 +2073,9 @@ qemuAgentGetInterfaceOneAddress(virDomainIPAddressPtr ip_addr, _("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 { + } + + if (STRNEQ(type, "ipv4") && STRNEQ(type, "ipv6")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown ip address type '%s'"), type); @@ -2091,7 +2089,6 @@ qemuAgentGetInterfaceOneAddress(virDomainIPAddressPtr ip_addr, " field for interface '%s'"), name); return -1; } - ip_addr->addr = g_strdup(addr); if (virJSONValueObjectGetNumberUint(ip_addr_obj, "prefix", &ip_addr->prefix) < 0) { @@ -2100,6 +2097,12 @@ qemuAgentGetInterfaceOneAddress(virDomainIPAddressPtr ip_addr, return -1; } + if (STREQ(type, "ipv4")) + ip_addr->type = VIR_IP_ADDR_TYPE_IPV4; + else + ip_addr->type = VIR_IP_ADDR_TYPE_IPV6; + + ip_addr->addr = g_strdup(addr); return 0; } -- 2.26.2