From: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>

Removing redundant sections of the code

Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
[ dannf: Add missing free of *pfname in error path and use proper < 0
  comparison when checking return for errors, both suggested by
  Michal Privoznik ]
Signed-off-by: dann frazier <dann.frazier@canonical.com>
---
 src/util/virnetdev.c | 36 ++++++------------------------------
 1 file changed, 6 insertions(+), 30 deletions(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 82823c0dfc..5571a10865 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1443,33 +1443,21 @@ int
 virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,
                                 int *vf)
 {
-    char *pf_sysfs_path = NULL, *vf_sysfs_path = NULL;
-    int ret = -1;
-
+    int ret;
     *pfname = NULL;
 
     if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Cannot get PF netdev name for VF %s"),
                        vfname);
-        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 = virNetDevGetVirtualFunctionIndex(*pfname, vfname, vf);
 
-    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;
 }
 
@@ -1865,13 +1853,9 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
          * it to PF + VFname
          */
 
-        if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
+        if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf) < 0)
             goto cleanup;
-
         pfDevName = pfDevOrig;
-
-        if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
-            goto cleanup;
     }
 
     if (pfDevName) {
@@ -2023,13 +2007,9 @@ virNetDevReadNetConfig(const char *linkdev, int vf,
          * it to PF + VFname
          */
 
-        if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
+        if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf) < 0)
             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
@@ -2228,13 +2208,9 @@ virNetDevSetNetConfig(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;
     }
 
 
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 1/22/19 8:26 PM, dann frazier wrote:
> From: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
> 
> Removing redundant sections of the code
> 
> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
> [ dannf: Add missing free of *pfname in error path and use proper < 0
>   comparison when checking return for errors, both suggested by
>   Michal Privoznik ]
> Signed-off-by: dann frazier <dann.frazier@canonical.com>
> ---
>  src/util/virnetdev.c | 36 ++++++------------------------------
>  1 file changed, 6 insertions(+), 30 deletions(-)
> 
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 82823c0dfc..5571a10865 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1443,33 +1443,21 @@ int
>  virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,
>                                  int *vf)
>  {
> -    char *pf_sysfs_path = NULL, *vf_sysfs_path = NULL;
> -    int ret = -1;
> -
> +    int ret;
>      *pfname = NULL;
>  
>      if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Cannot get PF netdev name for VF %s"),
>                         vfname);
> -        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 = virNetDevGetVirtualFunctionIndex(*pfname, vfname, vf);
>  
> -    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;

Nitpick, for better future extensibility I prefer initializing @ret,
keeping cleanup label and using goto cleanup.

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list