Re: [PATCH v1] virdnsmasq: fix runtime search for executable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 11, 2021 at 11:47:13AM +0200, Olaf Hering wrote:
> dnsmasq is an optional binary which does not neccessary exist during build.
>
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> ---
>  src/util/virdnsmasq.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)

Is there any error or incorrect behavior that you are trying to fix?
I did audit the code and within the virdnsmasq.c file we use the
binaryPath with virCommandRun() which will do this same check before
acutally execing, see virExec().

Pavel

> diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
> index f2f606913f..06d192c99d 100644
> --- a/src/util/virdnsmasq.c
> +++ b/src/util/virdnsmasq.c
> @@ -729,8 +729,26 @@ dnsmasqCapsRefreshInternal(dnsmasqCaps *caps, bool force)
>      return ret;
>  }
>  
> +static char *
> +dnsmasqGetBinaryPath(void)
> +{
> +    static const char binary[] = DNSMASQ;
> +    char *binary_path;
> +
> +    if (g_path_is_absolute(binary))
> +        return g_strdup(binary);
> +

No need for this check, virFindFileInPath calls g_find_program_in_path
which will do the correct thing if the path is already absolute.

> +    binary_path = virFindFileInPath(binary);
> +    if (!binary_path) {
> +        virReportSystemError(ENOENT, _("Cannot find '%s' in path"), binary);
> +        binary_path = g_strdup(binary);
> +    }
> +
> +    return binary_path;
> +}
> +
>  static dnsmasqCaps *
> -dnsmasqCapsNewEmpty(const char *binaryPath)
> +dnsmasqCapsNewEmpty(void)
>  {
>      dnsmasqCaps *caps;
>  
> @@ -739,14 +757,14 @@ dnsmasqCapsNewEmpty(const char *binaryPath)
>      if (!(caps = virObjectNew(dnsmasqCapsClass)))
>          return NULL;
>      caps->flags = virBitmapNew(DNSMASQ_CAPS_LAST);
> -    caps->binaryPath = g_strdup(binaryPath ? binaryPath : DNSMASQ);
> +    caps->binaryPath = dnsmasqGetBinaryPath();
>      return caps;
>  }
>  
>  dnsmasqCaps *
>  dnsmasqCapsNewFromBuffer(const char *buf)
>  {
> -    dnsmasqCaps *caps = dnsmasqCapsNewEmpty(DNSMASQ);
> +    dnsmasqCaps *caps = dnsmasqCapsNewEmpty();
>  
>      if (!caps)
>          return NULL;
> @@ -761,7 +779,7 @@ dnsmasqCapsNewFromBuffer(const char *buf)
>  dnsmasqCaps *
>  dnsmasqCapsNewFromBinary(void)
>  {
> -    dnsmasqCaps *caps = dnsmasqCapsNewEmpty(DNSMASQ);
> +    dnsmasqCaps *caps = dnsmasqCapsNewEmpty();
>  
>      if (!caps)
>          return NULL;
> @@ -776,7 +794,7 @@ dnsmasqCapsNewFromBinary(void)
>  const char *
>  dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps)
>  {
> -    return caps ? caps->binaryPath : DNSMASQ;
> +    return caps ? caps->binaryPath : dnsmasqGetBinaryPath();
>  }
>  
>  unsigned long
> 

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux