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 <rstoyan...@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

Reply via email to