Re: [libvirt] [PATCH] dnsmasq: Fix parsing of the version number
On 06.12.2012 13:29, Laine Stump wrote:
On 12/06/2012 06:34 AM, Michal Privoznik wrote:
> If the debugging is enabled, the virCommand subsystem catches debug
> messages in the command output as well. In that case, we can't assume
> the string corresponding to command's stdout will start with specific
> prefix. But the prefix can be moved deeper in the string. This bug
> shows itself when parsing dnsmasq output:
>
> 2012-12-06 11:18:11.445+0000: 18491: error :
> dnsmasqCapsSetFromBuffer:664 : internal error cannot parse
> /usr/sbin/dnsmasq version number in '2012-12-06 11:11:02.232+0000:
> 18492: debug : virFileClose:72 : Closed fd 22'
>
> We can clearly see that the output of dnsmasq --version
> doesn't start with expected "Dnsmasq version " string but a libvirt
> debug output.
This is a bug in virCommand, and should also affect
qemuCapsParseHelpStr(). Is there no way to fix it at the source, rather
than working around it like this?
Oh, right. The reason why this doesn't get exposed with
qemuCapsParseHelpStr() is - we are not keeping stderr when executing
'qemu --help' but we are doing that for dnsmasq. So I guess this patch
is obsolete and should be instead replaced by:
diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
index 4f210d2..bee3b61 100644
--- a/src/util/dnsmasq.c
+++ b/src/util/dnsmasq.c
@@ -715,7 +715,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool
force)
cmd = virCommandNewArgList(caps->binaryPath, "--version", NULL);
virCommandSetOutputBuffer(cmd, &version);
- virCommandSetErrorBuffer(cmd, &version);
virCommandAddEnvPassCommon(cmd);
virCommandClearCaps(cmd);
if (virCommandRun(cmd, NULL) < 0) {
@@ -727,7 +726,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool
force)
cmd = virCommandNewArgList(caps->binaryPath, "--help", NULL);
virCommandSetOutputBuffer(cmd, &help);
- virCommandSetErrorBuffer(cmd, &help);
virCommandAddEnvPassCommon(cmd);
virCommandClearCaps(cmd);
if (virCommandRun(cmd, NULL) < 0) {
> ---
> src/util/dnsmasq.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
> index 4f210d2..de0293a 100644
> --- a/src/util/dnsmasq.c
> +++ b/src/util/dnsmasq.c
> @@ -641,9 +641,9 @@ dnsmasqCapsSetFromBuffer(dnsmasqCapsPtr caps, const char *buf)
>
> caps->noRefresh = true;
>
> - p = STRSKIP(buf, DNSMASQ_VERSION_STR);
> - if (!p)
> + if (!(p = strstr(buf, DNSMASQ_VERSION_STR)))
> goto fail;
> + p += sizeof(DNSMASQ_VERSION_STR) - 1;
Why -1 ? That happens to work, but wouldn't if the version string didn't
end with a space.
sizeof(const char[]) counts '\0' at the end. We don't want that here.
> virSkipSpaces(&p);
> if (virParseVersionString(p, &caps->version, true) < 0)
> goto fail;
I'd rather make sure that nobody has a bright idea on how to fix
virCommand before pushing this.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list