On Mon, Apr 28, 2008 at 03:17:33AM -0400, Daniel Veillard wrote:
On Sat, Apr 26, 2008 at 05:37:11PM +0100, Daniel P. Berrange wrote:
The following set of changes adjust the way errors are handled in the virBuffer routines. The key idea is to make it hard (impossible) to misuse the API, with each change addressing one of the errors scenarios I've found in existing code using the routines.
In general I agree an like the change, except
- The contents of the struct are no longer public.
Rationale: This stops people accessing the buffer directly, thus preventing use of data which may be in an error state.
results in this:
struct _virBuffer { - char *content; /* The buffer content UTF8 */ - unsigned int use; /* The buffer size used */ - unsigned int size; /* The buffer size */ + char *padding[__SIZEOF_VIR_BUFFER]; /* This struct contents is private */ };
which is really not nice. 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; 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. Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|