Daniel Veillard wrote:
Enclosed is a cleanup patch I applied, it corrects the XML api generation, fixes warning in the XSLT stylesheet, add comments to a number of internal functions, makes most function of the QEmu back-end static, fixes a few bugs found in the way.
Looks fine, except my general grumble below.
@@ -113,12 +128,19 @@ virBufferFree(virBufferPtr buf) * virBufferContentAndFree: * @buf: Buffer * - * Return the content from the buffer and free (only) the buffer structure. + * Get the content from the buffer and free (only) the buffer structure. + * + * Returns the buffer content or NULL in case of error. */ char * virBufferContentAndFree (virBufferPtr buf) { - char *content = buf->content; + char *content; + + if (buf == NULL) + return(NULL); + + content = buf->content;
free (buf); return content;
I know we do this sort of thing all over the place, but it's bad practice (IMHO). If someone passes a NULL into this function then it's an error, and it's better to segfault early rather than compound or hide the error. Of course I wish we were using a language where you could specify these rules statically, and in fact I've been analysing the libvirt code using some static analysis tools to try & find these types of bugs automatically. Results later ... Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903