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