[libvirt] [PATCH v2 2/4] util: Code simplification

[Michal Privoznik]

On 11/17/18 8:51 PM, Radoslaw Biernacki wrote:
> Removing redundant sections of the code
> 
> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki at linaro.org>
> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/util/virnetdev.c | 40 +++-----------------
>  1 file changed, 5 insertions(+), 35 deletions(-)
> 
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index f1c2ba8c17..9318de406c 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1441,34 +1441,16 @@ int
>  virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,
>                                  int *vf)
>  {
> -    char *pf_sysfs_path = NULL, *vf_sysfs_path = NULL;
> -    int ret = -1;
> -
>      *pfname = NULL;
>  
>      if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Cannot get PF netdev name for VF %s"),
>                         vfname);

Sigh, this is overwriting an error reported by
virNetDevGetPhysicalFunction(). But it is pre-existing,

> -        return ret;
> +        return -1;
>      }
>  
> -    if (virNetDevSysfsFile(&pf_sysfs_path, *pfname, "device") < 0)
> -        goto cleanup;
> -
> -    if (virNetDevSysfsFile(&vf_sysfs_path, vfname, "device") < 0)
> -        goto cleanup;
> -
> -    ret = virPCIGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf);
> -
> - cleanup:
> -    if (ret < 0)
> -        VIR_FREE(*pfname);
> -
> -    VIR_FREE(vf_sysfs_path);
> -    VIR_FREE(pf_sysfs_path);
> -
> -    return ret;
> +    return virNetDevGetVirtualFunctionIndex(*pfname, vfname, vf);

Just realized that previously if there was an error then *pfname was
NULL upon returning from the function. Now it might be allocated. How about:

    ret = virNetDevGetVirtualFunctionIndex(*pfname, vfname, vf);

    if (ret < 0)
        VIR_FREE(*pfname);

    return ret;


>  }
>  
>  #else /* !__linux__ */
> @@ -1863,13 +1845,9 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
>           * it to PF + VFname
>           */
>  
> -        if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
> +        if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf))
>              goto cleanup;
> -
>          pfDevName = pfDevOrig;
> -
> -        if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
> -            goto cleanup;
>      }
>  
>      if (pfDevName) {
> @@ -2021,13 +1999,9 @@ virNetDevReadNetConfig(const char *linkdev, int vf,
>           * it to PF + VFname
>           */
>  
> -        if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
> +        if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf))

No, please keep the proper < 0 comparison.

>              goto cleanup;
> -
>          pfDevName = pfDevOrig;
> -
> -        if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
> -            goto cleanup;
>      }
>  
>      /* if there is a PF, it's now in pfDevName, and linkdev is either
> @@ -2226,13 +2200,9 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
>           * it to PF + VFname
>           */
>  
> -        if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
> +        if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf))

Here too.

>              goto cleanup;
> -
>          pfDevName = pfDevOrig;
> -
> -        if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
> -            goto cleanup;
>      }
>  
>  
> 

Michal