On 05/31/2013 09:54 AM, John Ferlan wrote: > Commit '6afdfc8e' adjusted the exit and error paths to go through the error > and cleanup labels, but neglected to remove the return ret prior to cleanup. > Also noted the 'type' xml string fetch was never checked for NULL which > could lead to some interesting results. > --- > src/conf/storage_conf.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c > index f0ea41d..94eb69a 100644 > --- a/src/conf/storage_conf.c > +++ b/src/conf/storage_conf.c > @@ -834,6 +834,12 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) > } > > type = virXPathString("string(./@type)", ctxt); > + if (type == NULL) { > + virReportError(VIR_ERR_XML_ERROR, > + _("storage pool missing type attribute")); > + goto error; > + } > + > if ((ret->type = virStoragePoolTypeFromString(type)) < 0) { > virReportError(VIR_ERR_XML_ERROR, > _("unknown storage pool type %s"), type); > @@ -956,8 +962,6 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) > goto error; > } > > - return ret; > - > cleanup: > VIR_FREE(uuid); > VIR_FREE(type); > Peter also pointed out I was missing the "%s" in the new type check error path. Something syntax-check would have caught (of course I had run that on my other patch sent earlier, but forgot to on this one, sigh). In any case, with the "%s" added, syntax-check passing, and rerun of valgrind, I pushed. Thanks, John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list