[libvirt] [PATCH] Remove redundant virFileIsExecutable check
Radostin Stoyanov
rstoyanov1 at gmail.com
Fri Apr 13 08:10:02 UTC 2018
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 at 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
More information about the libvir-list
mailing list