[libvirt] [PATCH 7/8] interface: Clean up virInterfaceObjListFindByMACString

[Michal Privoznik]

On 04/26/2017 12:36 AM, John Ferlan wrote:
> Alter the algorithm to return a list of matching names rather than a
> list of match virInterfaceObjPtr which are then just dereferenced
> extracting the def->name and def->mac. Since the def->mac would be
> the same as the passed @mac, just return a list of names and as long
> as there's only one, extract the [0] entry from the passed list.
> Also alter the error message on failure to include the mac that wasn't
> found.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/conf/virinterfaceobj.c | 23 ++++++++++++++---------
>  src/conf/virinterfaceobj.h |  2 +-
>  src/test/test_driver.c     | 16 ++++++++--------
>  3 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
> index 3aeeebd..1cc5c92 100644
> --- a/src/conf/virinterfaceobj.c
> +++ b/src/conf/virinterfaceobj.c
> @@ -108,11 +108,11 @@ virInterfaceObjListNew(void)
>  int
>  virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>                                     const char *mac,
> -                                   virInterfaceObjPtr *matches,
> +                                   char **const matches,
>                                     int maxmatches)
>  {
>      size_t i;
> -    unsigned int matchct = 0;
> +    int matchct = 0;
>  
>      for (i = 0; i < interfaces->count; i++) {
>          virInterfaceObjPtr obj = interfaces->objs[i];
> @@ -121,18 +121,23 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>          virInterfaceObjLock(obj);
>          def = obj->def;
>          if (STRCASEEQ(def->mac, mac)) {
> -            matchct++;
> -            if (matchct <= maxmatches) {
> -                matches[matchct - 1] = obj;
> -                /* keep the lock if we're returning object to caller */
> -                /* it is the caller's responsibility to unlock *all* matches */
> -                continue;
> +            if (matchct < maxmatches) {
> +                if (VIR_STRDUP(matches[matchct], def->name) < 0) {
> +                    virInterfaceObjUnlock(obj);
> +                    goto error;
> +                }
> +                matchct++;
>              }
>          }
>          virInterfaceObjUnlock(obj);
> -
>      }
>      return matchct;
> +
> + error:
> +    while (--matchct >= 0)
> +        VIR_FREE(matches[matchct]);
> +
> +    return -1;
>  }
>  
>  
> diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
> index f1bcab9..3934e63 100644
> --- a/src/conf/virinterfaceobj.h
> +++ b/src/conf/virinterfaceobj.h
> @@ -44,7 +44,7 @@ virInterfaceObjListNew(void);
>  int
>  virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>                                     const char *mac,
> -                                   virInterfaceObjPtr *matches,
> +                                   char **const matches,
>                                     int maxmatches);
>  
>  virInterfaceObjPtr
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 89a705c..ac16f4f 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -3728,17 +3728,18 @@ testInterfaceLookupByMACString(virConnectPtr conn,
>                                 const char *mac)
>  {
>      testDriverPtr privconn = conn->privateData;
> -    virInterfaceObjPtr obj;
> -    virInterfaceDefPtr def;
>      int ifacect;
> +    char *ifacenames[] = { NULL, NULL };
>      virInterfacePtr ret = NULL;
>  
>      testDriverLock(privconn);
> -    ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, &obj, 1);
> +    ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac,
> +                                                 ifacenames, 2);

ARRAY_CARDINALITY()

>      testDriverUnlock(privconn);
>  
>      if (ifacect == 0) {
> -        virReportError(VIR_ERR_NO_INTERFACE, NULL);
> +        virReportError(VIR_ERR_NO_INTERFACE,
> +                       _("no interface with matching mac '%s'"), mac);
>          goto cleanup;
>      }
>  
> @@ -3747,12 +3748,11 @@ testInterfaceLookupByMACString(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> -    def = virInterfaceObjGetDef(obj);
> -    ret = virGetInterface(conn, def->name, def->mac);
> +    ret = virGetInterface(conn, ifacenames[0], mac);
>  
>   cleanup:
> -    if (obj)
> -        virInterfaceObjUnlock(obj);
> +    VIR_FREE(ifacenames[0]);
> +    VIR_FREE(ifacenames[1]);

And this can be probably dynamic then:

for (i = 0; i < ARRAY_CARDINALITY(ifacenames); i++)
    VIR_FREE(ifacenames[i]);

>      return ret;
>  }
>  
> 

Michal