1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] Remove redundant virFileIsExecutable check

Radostin Stoyanov posted 1 patch 6 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180413060145.10659-1-rstoyanov1@gmail.com
Test syntax-check passed
src/bhyve/bhyve_capabilities.c | 4 ----
src/qemu/qemu_capabilities.c   | 2 +-
2 files changed, 1 insertion(+), 5 deletions(-)
[libvirt] [PATCH] Remove redundant virFileIsExecutable check
Posted by Radostin Stoyanov 6 years ago 
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@gmail.com>
---
 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;
 
     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);
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Remove redundant virFileIsExecutable check
Posted by Michal Privoznik 6 years ago 
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@gmail.com>
> ---
>  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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Remove redundant virFileIsExecutable check
Posted by Radostin Stoyanov 6 years ago 
On 13/04/18 08:35, Michal Privoznik wrote:
> 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@gmail.com>
>> ---
>>  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.
Thanks :)

Radostin
> Michal

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.