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

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to