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

Re: [libvirt] [PATCH 2/8] Add qemuMonitorGetObjectProperty() method for QMP qom-get command



On 07/03/2013 08:20 AM, Michal Privoznik wrote:
> On 03.07.2013 14:12, Ján Tomko wrote:
>> On 07/03/2013 02:03 PM, Michal Privoznik wrote:
>>> On 02.07.2013 15:39, John Ferlan wrote:
>>>> +    case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
>>>> +        tmp = virJSONValueGetString(data);
>>>> +        if (tmp && VIR_STRDUP(prop->val.str, tmp) < 0)
>>>
>>> Lose the 'tmp' here as VIR_STRDUP will check it for NULL anyway. In
>>> fact, these lines can be rewritten into a single line:
>>>
>>> ret = VIR_STRDUP(prop->val.str, tmp);
>>>
>>>> +            goto cleanup;
>>>> +        if (tmp)
>>>> +            ret = 0;
>>>> +        break;
>>
>> Not at all. Before it returns 0 if GetString returned something.
>> After your change, ret will be 1 in that case, and 0 if GetString returned
>> nothing.
>>
>> Jan
>>
> 
> But if you take a look a few lines below, we will rewrite ret = 0
> anyway. The only exception is if VIR_STRDUP fails. Which I should have
> checked, right.
> 
> 
> Michal
> 

I didn't want VIR_STRDUP() to be called if 'data' wasn't a string.  That
is - I don't want an empty "" returned.  That allows the error to be
reported that there's invalid data.

Although it seems the qemuMonitorJSONHumanCommandWithFd() does an
extraneous check for VIR_STRDUP():

        data = virJSONValueGetString(obj);
        if (VIR_STRDUP(*reply_str, data ? data : "") < 0)
            goto cleanup;

Not part of this, but I was checking callers for virJSONValueGetString()

John


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