On Fri, Jun 29, 2007 at 02:40:23PM +0100, Richard W.M. Jones wrote:
@@ -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.
The error should be raised before. If it wasn't then there is a programming bug. And a library must not segfault on hazard like out of memory, sorry no ! Like we tests return from malloc() I think we must shield the internal code as much as possible. In any case segfaulting is not the proper way to raise a problem.
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 ...
Fixing bugs, yes definitely, that I can only agree with. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/