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@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. Thanks :) Radostin > Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list