[libvirt] [PATCH] util: Fix return value for virJSONValueFromString if it fails

[Osier Yang]

于 2011年03月23日 21:19, Daniel P. Berrange 写道:
> On Wed, Mar 23, 2011 at 08:07:25PM +0800, Osier Yang wrote:
>> Problem:
>>    "parser.head" is not NULL even if it's free'ed by "virJSONValueFree",
>> returning "parser.head" when "virJSONValueFromString" fails will cause
>> unexpected errors (libvirtd will crash sometimes), e.g.
>>    In function "qemuMonitorJSONArbitraryCommand":
>>
>>          if (!(cmd = virJSONValueFromString(cmd_str)))
>>              goto cleanup;
>>
>>          if (qemuMonitorJSONCommand(mon, cmd,&reply)<  0)
>>              goto cleanup;
>>
>>          ......
>>
>>       cleanup:
>>          virJSONValueFree(cmd);
>>
>>    It will continues to send command to monitor even if "virJSONValueFromString"
>> is failed, and more worse, it trys to free "cmd" again.
>>
>>    Crash example:
>> {"error":{"class":"QMPBadInputObject","desc":"Expected 'execute' in QMP input","data":{"expected":"execute"}}}
>> {"error":{"class":"QMPBadInputObject","desc":"Expected 'execute' in QMP input","data":{"expected":"execute"}}}
>> error: server closed connection:
>> error: unable to connect to '/var/run/libvirt/libvirt-sock', libvirtd may need to be started: Connection refused
>> error: failed to connect to the hypervisor
>>
>>    This fix is to:
>>      1) return NULL for failure of "virJSONValueFromString",
>>      2) and it seems "virJSONValueFree" uses incorrect loop index for type
>>         of "VIR_JSON_TYPE_OBJECT", fix it together.
>>
>> * src/util/json.c
>> ---
>>   src/util/json.c |    7 +++++--
>>   1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/util/json.c b/src/util/json.c
>> index f90594c..be47f64 100644
>> --- a/src/util/json.c
>> +++ b/src/util/json.c
>> @@ -65,7 +65,7 @@ void virJSONValueFree(virJSONValuePtr value)
>>
>>       switch (value->type) {
>>       case VIR_JSON_TYPE_OBJECT:
>> -        for (i = 0 ; i<  value->data.array.nvalues ; i++) {
>> +        for (i = 0 ; i<  value->data.object.npairs; i++) {
>>               VIR_FREE(value->data.object.pairs[i].key);
>>               virJSONValueFree(value->data.object.pairs[i].value);
>>           }
>> @@ -897,6 +897,7 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring)
>>       yajl_parser_config cfg = { 1, 1 };
>>       yajl_handle hand;
>>       virJSONParser parser = { NULL, NULL, 0 };
>> +    virJSONValuePtr ret = NULL;
>>
>>       VIR_DEBUG("string=%s", jsonstring);
>>
>> @@ -917,6 +918,8 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring)
>>           goto cleanup;
>>       }
>>
>> +    ret = parser.head;
>> +
>>   cleanup:
>>       yajl_free(hand);
>>
>> @@ -930,7 +933,7 @@ cleanup:
>>
>>       VIR_DEBUG("result=%p", parser.head);
>>
>> -    return parser.head;
>> +    return ret;
>>   }
>
> ACK
>
> Daniel

Thanks, pushed.

Regards
Osier