[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] PATCH: Make the virBuffer API harder to misuse



On Mon, Apr 28, 2008 at 02:59:06PM +0100, Daniel P. Berrange wrote:
> On Mon, Apr 28, 2008 at 08:19:20AM -0400, Daniel Veillard wrote:
> > On Mon, Apr 28, 2008 at 01:13:00PM +0100, Daniel P. Berrange wrote:
> > > On Mon, Apr 28, 2008 at 03:17:33AM -0400, Daniel Veillard wrote:
> > > > I would prefer to relax the 'non-public' point and let the compiler 
> > > > compute the size in some ways rather than hardcode based on a word size
> > > > indication which may not take into account specific alignment problems
> > > > on some platforms.
> > > 
> > > One other option I considered is to just define the struct in the public 
> > > header with meaningless field names
> > > 
> > >   struct _virBuffer {
> > >      char a;
> > 
> >        char *a;
> > 
> > >      unsigned int b;
> > >      unsinged int c;
> > >   };
> > > 
> > > The real version is re-declared with proper names in buf.c, so this will
> > > at least discourage its use which is probably good enough.
> > 
> >   Fine by me.
> 
> Here's the updated patch...

  Okay, that's massive, best to apply it early since we agree on it.

> --- src/buf.c	10 Apr 2008 16:54:54 -0000	1.15
> +++ src/buf.c	28 Apr 2008 13:52:15 -0000
> @@ -18,8 +18,37 @@
>  #include <stdarg.h>
>  #include <ctype.h>
>  
> +#define __VIR_BUFFER_C__
> +
>  #include "buf.h"
>  
> +
> +/* If adding more fields, ensure to edit buf.h to match
> +   the number of fields */
> +struct _virBuffer {
> +    unsigned int size;
> +    unsigned int use;
> +    unsigned int error;
> +    char *content;
> +};

  I would still prefer if we could get the compiler to check the
structure (or at least their size) is the same... but that's should not
impact how the client code will be using the buffers so, we can try to
fix that after the main commit.

> +/**
> + * virBufferFail

  * virBufferNoMemory:

> + * @buf: the buffer
> + *
> + * Mark the buffer has having failed a memory allocation,
> + * freeing the content and setting the error flag.
> + */
> +static void
> +virBufferNoMemory(const virBufferPtr buf)
> +{

  thanks,

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/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]