[libvirt PATCH 05/14] qemu: agent: expand addrs upfront

[Jonathon Jongsma]

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

> 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 at redhat.com>
> ---
>  src/qemu/qemu_agent.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index c6878c8590..dc989622b9 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -2213,20 +2213,17 @@ 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;
> +

I don't see much benefit to keeping the local "addrs_count" variable
here. Previously this local var was incremented within the loop and then
used to set iface->naddrs after all iterations of the loop were
completed. But you're no longer doing anything with it other than
setting it right here. The iface->naddrs value is incremented separately
within the loop.  It should be safe to just set iface->naddrs here
since any newly allocated elements are guaranteed to be filled with
zeros.

>          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++; 

Then you could just use 'iface->addrs + j' here instead of incrementing
the naddrs variable (which IMO reduces the readability of the code).

>              if (qemuAgentGetInterfaceOneAddress(ip_addr,
> ip_addr_obj, name) < 0) goto error;
>          }
> -
> -        iface->naddrs = addrs_count;
>      }
>  
>      *ifaces = g_steal_pointer(&ifaces_ret);