[libvirt] [PATCH] dnsmasq: Fix parsing of the version number

[Michal Privoznik]

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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>