Re: [PATCH] Remove redundant virFileIsExecutable check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/13/2018 08:01 AM, Radostin Stoyanov wrote:
> Remove unnecessary virFileIsExecutable check after virFindFileInPath.
> Since the commit 9ae992f virFindFileInPath will reject non-executables.
> 
> 9ae992f24353d6506f570fc9dd58355b165e4472
> virFindFileInPath: only find executable non-directory
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@xxxxxxxxx>
> ---
>  src/bhyve/bhyve_capabilities.c | 4 ----
>  src/qemu/qemu_capabilities.c   | 2 +-
>  2 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c
> index 381cc0de3..e13085b1d 100644
> --- a/src/bhyve/bhyve_capabilities.c
> +++ b/src/bhyve/bhyve_capabilities.c
> @@ -179,8 +179,6 @@ virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps)
>      binary = virFindFileInPath("grub-bhyve");
>      if (binary == NULL)
>          goto out;
> -    if (!virFileIsExecutable(binary))
> -        goto out;
>  
>      cmd = virCommandNew(binary);
>      virCommandAddArg(cmd, "--help");
> @@ -315,8 +313,6 @@ virBhyveProbeCaps(unsigned int *caps)
>      binary = virFindFileInPath("bhyve");
>      if (binary == NULL)
>          goto out;
> -    if (!virFileIsExecutable(binary))
> -        goto out;

These twp ^^ make sense.

>  
>      if ((ret = bhyveProbeCapsRTC_UTC(caps, binary)))
>          goto out;
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 27180e850..5ebc72f6f 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -653,7 +653,7 @@ virQEMUCapsFindBinary(const char *format,
>  
>      ret = virFindFileInPath(binary);
>      VIR_FREE(binary);
> -    if (ret && virFileIsExecutable(ret))
> +    if (ret == NULL)
>          goto out;
>  
>      VIR_FREE(ret);
> 

However, this one does not. With this change, if virFindFileInPath()
returned something, VIR_FREE() is called over it and NULL is returned.
So the condition should be reversed. But looking at whole function, it's
insane code and the if() statement is not needed at all.

I'm squashing this in:

diff --git i/src/qemu/qemu_capabilities.c w/src/qemu/qemu_capabilities.c
index 5ebc72f6f4..c8488f875d 100644
--- i/src/qemu/qemu_capabilities.c
+++ w/src/qemu/qemu_capabilities.c
@@ -649,16 +649,10 @@ virQEMUCapsFindBinary(const char *format,
     char *binary = NULL;
 
     if (virAsprintf(&binary, format, archstr) < 0)
-        goto out;
+        return NULL;
 
     ret = virFindFileInPath(binary);
     VIR_FREE(binary);
-    if (ret == NULL)
-        goto out;
-
-    VIR_FREE(ret);
-
- out:
     return ret;
 }


ACKing and pushing.

Michal

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux