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
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature