Re: [libvirt] [PATCH] Remove redundant virFileIsExecutable check

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(a)gmail.com&gt; > --- > 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