On Mon, Nov 09, 2009 at 12:07:40PM +0100, Matthias Bolte wrote:
2009/11/9 Daniel Veillard <veillard@redhat.com>:
On Sun, Nov 08, 2009 at 10:40:42PM +0100, Matthias Bolte wrote:
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index c2c5a44..c5083cc 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1057,13 +1057,18 @@ virNodeDeviceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt, int create /* Extract device name */ if (create == EXISTING_DEVICE) { def->name = virXPathString(conn, "string(./name[1])", ctxt); + + if (!def->name) { + virNodeDeviceReportError(conn, VIR_ERR_NO_NAME, NULL); + goto error; + } } else { def->name = strdup("new device"); - }
- if (!def->name) { - virNodeDeviceReportError(conn, VIR_ERR_NO_NAME, NULL); - goto error; + if (!def->name) { + virReportOOMError(conn); + goto error; + } }
/* Extract device parent, if any */
I disagree with this one. The XPath string(./name[1]) can fail without this being an allocation error, it mays just be that there is no name element at the current node, and current behaviour looks better to me. Moreover if virXPathString() returns NULL because of a string allocation error it will already raise an OOMError
I think you misread this one. The original code assigns the result of virXPathString() or strdup() to def->name. After that it checks def->name for NULL and reports an no-name error even if the NULL was returned by strdup(), indicating an OOM error.
Whoops, right :-) Rereading the patch it's fine !
I moved the no-name error report into the virXPathString() case and added an OOM error in the strdup() case. [...]
So you had to add a filed in the iterator structure to report OOMs while running the iterator, nice work !
Well, DPB used this pattern in his "Convert virDomainObjListPtr to use a hash of domain objects" patch, I just applied it here too :-)
ACK except for the one in virNodeDeviceDefParseXML(), very good job you must have spent a lot of time, thanks a lot !
Daniel
It took some hours, but the task was simple: search and fix.
Well thanks a lot ! and ACK :-) Danie -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/