- Devel - libvirt List Archives

[Libvir] also check for useless test-before-xmlFree
by Jim Meyering 29 Apr '08

29 Apr '08

Some time ago, Daniel Veillard assured me that xmlFree(NULL) is valid in regular use, and with upstream code since January it's ok even in a debug mode that's not normally available because it's ifdef'd out: http://mail.gnome.org/archives/svn-commits-list/2008-January/msg02233.html Since the potential NULL-deref in debug mode is now gone, we can now use this to prevent any new useless tests: also check for useless test-before-xmlFree * Makefile.cfg (useless_free_options): Add --name=xmlFree. diff --git a/Makefile.cfg b/Makefile.cfg index e0d5528..cbf97c2 100644 --- a/Makefile.cfg +++ b/Makefile.cfg @@ -53,5 +53,6 @@ local-checks-to-skip = \ useless_free_options = \ --name=sexpr_free \ + --name=xmlFree \ --name=xmlXPathFreeContext \ --name=xmlXPathFreeObject -- 1.5.5.1.68.gbdcd8

2 1

[Libvir] PATCH: Avoid buffer out of bounds access in Xen capabilities
by Daniel P. Berrange 29 Apr '08

29 Apr '08

The Xen driver uses a regex to process the hypervisor capabilities data "(xen|hvm)-[[:digit:]]+\\.[[:digit:]]+-(x86_32|x86_64|ia64|powerpc64)(p|be)?"; notice how the last match group, however, is optional due to the '?'. The code processing matches does not check to see if the match is present or not, and just indexes the string on match 3 if (strncmp (&token[subs[3].rm_so], "p", 1) == 0) Unfortunately, subs[3].rm_so is -1 if the match was not present, so we're doing an out of bounds array access here. This is fairly harmless, but it is still good to fix it. So this patch adds a check for -1 before accessing the match. I also replace the strncmp() calls with a call to the brand new STRPREFIX() convenience macro Dan. Index: src/xen_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xen_internal.c,v retrieving revision 1.119 diff -u -r1.119 xen_internal.c --- src/xen_internal.c 10 Apr 2008 16:54:54 -0000 1.119 +++ src/xen_internal.c 28 Apr 2008 22:07:12 -0000 @@ -2349,28 +2349,31 @@ if (regexec (&xen_cap_rec, token, sizeof subs / sizeof subs[0], subs, 0) == 0) { - int hvm = strncmp (&token[subs[1].rm_so], "hvm", 3) == 0; + int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm"); const char *model; int bits, pae = 0, nonpae = 0, ia64_be = 0; - if (strncmp (&token[subs[2].rm_so], "x86_32", 6) == 0) { + + if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { model = "i686"; bits = 32; - if (strncmp (&token[subs[3].rm_so], "p", 1) == 0) + if (subs[3].rm_so != -1 && + STRPREFIX(&token[subs[3].rm_so], "p")) pae = 1; else nonpae = 1; } - else if (strncmp (&token[subs[2].rm_so], "x86_64", 6) == 0) { + else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) { model = "x86_64"; bits = 64; } - else if (strncmp (&token[subs[2].rm_so], "ia64", 4) == 0) { + else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) { model = "ia64"; bits = 64; - if (strncmp (&token[subs[3].rm_so], "be", 2) == 0) + if (subs[3].rm_so != -1 && + STRPREFIX(&token[subs[3].rm_so], "be")) ia64_be = 1; } - else if (strncmp (&token[subs[2].rm_so], "powerpc64", 4) == 0) { + else if (STRPREFIX(&token[subs[2].rm_so], "powerpc64")) { model = "ppc64"; bits = 64; } else { @@ -2380,7 +2383,7 @@ /* Search for existing matching (model,hvm) tuple */ for (i = 0 ; i < nr_guest_archs ; i++) { - if (!strcmp(guest_archs[i].model, model) && + if (STREQ(guest_archs[i].model, model) && guest_archs[i].hvm == hvm) { break; } -- |: 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 :|

2 2

[Libvir] [PATCH] Avoid "make syntax-check" failures.
by Jim Meyering 29 Apr '08

29 Apr '08

Including config.h from memory.c is probably required only on um, ... unusual systems, but technically, it is required for the definition of a possibly missing "size_t" or "ptrdiff_t" type. Avoid "make syntax-check" failures. * src/memory.c: Include "config.h". Remove trailing blanks. diff --git a/src/memory.c b/src/memory.c index fe41e38..1ca67fb 100644 --- a/src/memory.c +++ b/src/memory.c @@ -19,6 +19,7 @@ * */ +#include <config.h> #include <stdlib.h> #include "memory.h" @@ -72,7 +73,7 @@ int virAlloc(void *ptrptr, size_t size) * @size: number of bytes to allocate * @count: number of elements to allocate * - * Allocate an array of memory 'count' elements long, + * Allocate an array of memory 'count' elements long, * each with 'size' bytes. Return the address of the * allocated memory in 'ptrptr'. The newly allocated * memory is filled with zeros. @@ -101,7 +102,7 @@ int virAllocN(void *ptrptr, size_t size, size_t count) * Resize the block of memory in 'ptrptr' to be an array of * 'count' elements, each 'size' bytes in length. Update 'ptrptr' * with the address of the newly allocated memory. On failure, - * 'ptrptr' is not changed and still points to the original memory + * 'ptrptr' is not changed and still points to the original memory * block. The newly allocated memory is filled with zeros. * * Returns -1 on failure to allocate, zero on success -- 1.5.5.1.68.gbdcd8

2 1

[Libvir] [PATCH] Ancient libparted
by Soren Hansen 29 Apr '08

29 Apr '08

Some of us are stuck with an ancient libparted, which doesn't know about PED_PARTITION_PROTECTED. This patch allows us to compile libvirt. === modified file 'src/parthelper.c' --- src/parthelper.c 2008-04-10 16:53:29 +0000 +++ src/parthelper.c 2008-04-29 07:47:08 +0000 @@ -67,8 +67,10 @@ content = "free"; else if (part->type & PED_PARTITION_METADATA) content = "metadata"; +#ifdef PED_PARTITION_PROTECTED else if (part->type & PED_PARTITION_PROTECTED) content = "protected"; +#endif else content = "data"; } else if (part->type == PED_PARTITION_EXTENDED) { @@ -80,8 +82,10 @@ content = "free"; else if (part->type & PED_PARTITION_METADATA) content = "metadata"; +#ifdef PED_PARTITION_PROTECTED else if (part->type & PED_PARTITION_PROTECTED) content = "protected"; +#endif else content = "data"; } -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/

4 3

[Libvir] [PATCH] Allow selection of the NIC model in QEMU/KVM
by Richard W.M. Jones 29 Apr '08

29 Apr '08

This patch allows selection of the NIC model for QEMU/KVM domains. The selection is done by adding a <model/> element to the XML, as in this example: <interface type='user'> <mac address='00:16:3e:33:b8:d3'/> <model type='ne2k_pci'/> </interface> The model type string is only checked to make sure it's a short alpha-numeric + underscore, since it seems impractical to extract the actual list of supported models. If you choose a supported model then QEMU starts up with this extra -nic parameter: /usr/bin/qemu-kvm -M pc -m 500 -smp 1 -monitor pty \ -boot c -hda /var/lib/xen/images/rhel51x32kvm.img \ -net nic,macaddr=00:16:3e:33:b8:d3,vlan=0,model=ne2k_pci -net user,vlan=0 \ -usb -vnc 127.0.0.1:0 If you choose a non-existant model then you get the error: libvir: QEMU error : internal error QEMU quit during monitor startup Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top

6 8

[Libvir] PATCH: Add docs on the network XML format
by Daniel P. Berrange 29 Apr '08

29 Apr '08

This patch fills in the network driver XML format page on the website which has been requested many times... Dan. Index: formatnetwork.html.in =================================================================== RCS file: /data/cvs/libvirt/docs/formatnetwork.html.in,v retrieving revision 1.1 diff -r1.1 formatnetwork.html.in 4a5,122 > <p> > This page provides an introduction to the network XML format. For background > information on the concepts referred to here, consult the <a href="archnetwork.html">network driver architecture</a> > page. > </p> > > <h2>Element and attribute overview</h2> > > <p> > The root element required for all virtual networks is > named <code>network</code> and has no attributes. > </p> > > <h3>General metadata</h3> > > <p> > The first elements provide basic metadata about the virtual > network. > </p> > > <pre> > <network> > <name>default</name> > <uuid>3e3fce45-4f53-4fa7-bb32-11f34168b82b</uuid> > ...</pre> > > <dl> > <dt><code>name</code></dt> > <dd>The content of the <code>name</code> element provides > a short name for the virtual network. This name should > consist only of alpha-numeric characters and is required > to be unique within the scope of a single host. It is > used to form the filename for storing the persistent > configuration file.</dd> > <dt><code>uuid</code></dt> > <dd>The content of the <code>uuid</code> element provides > a globally unique identifier for the virtual network. > The format must be RFC 4122 compliant, eg <code>3e3fce45-4f53-4fa7-bb32-11f34168b82b</code>. > If omitted when defining/creating a new network, a random > UUID is generated.</dd> > </dl> > > <h3>Connectivity</h3> > > <p> > The next set of elements control how a virtual network is > provided connectivity to the physical LAN (if at all). > </p> > > <pre> > ... > <bridge name="virbr0" /> > <forward type="nat"/> > ...</pre> > > <dl> > <dt><code>bridge</code></dt> > <dd>The <code>name</code> attribute on the <code>bridge</code> element > defines the name of a bridge device which will be used to construct > the virtual network. The virtual machines will be connected to this > bridge device allowing them to talk to each other. The bridge device > may also be connected to the LAN. It is recommended that bridge > device names started with the prefix <code>vir</code>, but the name > <code>virbr0</code> is reserved for the "default" virtual network. > This element should always be provided when defining a new network > </dd> > <dt><code>forward</code></dt> > <dd>Inclusion of the <code>forward</code> element indicates that > the virtual network is to be connected to the physical LAN. If > no attributes are set, NAT forwarding will be used for connectivity. > Firewall rules will allow forwarding to any other network device whether > ethernet, wireless, dialup, or VPN. If the <code>dev</code> attribute > is set, the firewall rules will restrict forwarding to the named > device only. If the <code>type</code> attribute is set to <code>route</code> > then the traffic will not have NAT applied. This presumes that the > local LAN router has suitable routing table entries to return traffic > to this host.</dd> > </dl> > > <h3>Addressing</h3> > > <p> > The final set of elements define the IPv4 address range available, > and optionally enable DHCP sevices. > </p> > > <pre> > ... > <ip address="192.168.122.1" netmask="255.255.255.0"> > <dhcp> > <range start="192.168.122.2" end="192.168.122.254" /> > </dhcp> > </ip> > </network></pre> > > <dl> > <dt><code>ip</code></dt> > <dd>The <code>address</code> attribute defines an IPv4 address in > dotted-decimal format, that will be configured on the bridge > device associated with the virtual network. To the guests this > address will be their default route. The <code>netmask</code> > attribute defines the significant bits of the network address, > again specified in dotted-decimal format. > </dd> > <dt><code>dhcp</code></dt> > <dd>Immediately within the <code>ip</code> element there is an > optional <code>dhcp</code> element. The presence of this element > enables DHCP services on the virtual network. It will further > contain one or more <code>range</code> elements. > </dd> > <dt><code>range</code></dt> > <dd>The <code>start</code> and <code>end</code> attributes on the > <code>range</code> element specify the boundaries of a pool of > IPv4 addresses to be provided to DHCP clients. These two addresses > must lie within the scope of the network defined on the parent > <code>ip</code> element. > </dd> > </dl> 9a128,137 > <p> > This example is the so called "default" virtual network. It is > provided and enabled out-of-the-box for all libvirt installations. > This is a configuration that allows guest OS to get outbound > connectivity regardless of whether the host uses ethernet, wireless, > dialup, or VPN networking without requiring any specific admin > configuration. In the absence of host networking, it at least allows > guests to talk directly to each other. > </p> > 23a152,160 > <p> > This is a variant on the default network which routes traffic > from the virtual network to the LAN without applying any NAT. > It requires that the IP address range be pre-configured in the > routing tables of the router on the host network. This example > further specifies that guest traffic may only go out via the > <code>eth1</code> host network device. > </p> > 37a175,182 > <p> > This variant provides a completely isolated private network > for guests. The guests can talk to each other, and the host > OS, but cannot reach any other machines on the LAN, due to > the omission of the <code>forward</code> element in the XML > description. > </p> > -- |: 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 :|

2 1

[Libvir] RFC: safer memory allocation APIs with compile time checking
by Daniel P. Berrange 29 Apr '08

29 Apr '08

After updating the virBuffer APIs to protect against improper usage I have been thinking about how we might provider safer memory allocation APIs with protection against common usage errors and compile time validation of checks for failure. The standard C library malloc/realloc/calloc/free APIs have just about the worst possible design, positively encouraging serious application coding errors. There are at least 7 common problems I can think of at the moment, perhaps more.... 1. malloc() - pass incorrect number of bytes 2. malloc() - fail to check the return value 3. malloc() - use uninitialized memory 4. free() - double free by not setting pointer to NULL 5. realloc() - fail to check the return value 6. realloc() - fail to re-assign the pointer with new address 7. realloc() - accidental overwrite of original pointer with NULL on failure, leaking memory Many applications / libraries wrap these in their own functions, but typically fail to address one or more these problems. eg glib re-definitions try to address point 2 by having g_malloc() call abort() on failure, but then re-introduce the problem by adding g_try_malloc() gpointer g_malloc (gulong n_bytes) G_GNUC_MALLOC; gpointer g_try_malloc (gulong n_bytes) G_GNUC_MALLOC; Simiarly it tries address point 6, with the annotation, but this still leaves open the failure to check for NULL in point 1. gpointer g_realloc (gpointer mem, gulong n_bytes) G_GNUC_WARN_UNUSED_RESULT; gpointer g_try_realloc (gpointer mem, gulong n_bytes) G_GNUC_WARN_UNUSED_RESULT; And the g_free wrapper doesn't help with the double free issue at all: void g_free (gpointer mem); All 7 of the errors listed early could be avoided and/or checked at compile time if the APIs used a different calling convention. 1. For any given pointer the compiler knows how many bytes need to be allocated for a single instance of it. ie sizeof(*(ptr)). Since C has no data type representing a data type, this cannot be done with a simple function - it needs a (trivial) macro. 2. The __attribute__((__warn_unused_result__)) annotation causes the compiler to warn if an application doesn't do something with a return value. With the standard malloc() API this doesn't help because you always assign the return value to a pointer. The core problem here is that the API encodes the error condition as a special case of the actual data returned. This can be addressed by separating the two. The return value should simply be bi-state success or fail code and the return data passed back via an pointer to a pointer parameter. 3. The memory allocated by malloc() is not initialized to zero. For that a separate calloc() function is provided. This leaves open the possiblity of an app mistakenly using the wrong variant. Or consider an app using malloc() and explicitly initializing all struct members. Some time later a new member is added and now the developer needs to find all malloc() calls wrt to that struct and explicitly initilize the new member. It would be safer to always have malloc zero all memory, even though it has a small performance impact, the net win in safety is probably worth it. 4. Since free() takes the pointer to be freed directly it cannot reset the caller's original pointer back to NULL. This can be addressed if a pointer to the pointer is passed instead. The computational cost of the often redundant assignment to NULL is negligable given the safety benefit 5, 6 & 7. As with problem 2, these problems are all caused by the fact that the error condition is special case of the return data value. The impact of these is typically far worse because it often results in a buffer overflow and the complex rules for calling realloc() are hard to remember. So the core requirements of a safer API for allocation are: - Use return values *only* for the success/fail error condition - Annotate all the return values with __warn_unused_result__ - Pass a pointer to the pointer into all functions - Define macros around all functions to automatically do the sizeof(*(ptr)) Passing a pointer to a pointer gets a little tedious because of the need to write '&(foo)' instead of just 'foo', and is actually introduces a new problem of the caller accidentally passing merely a pointer, instead of a pointer to a pointer. This is a minor problem though because it will be identified on the very first run of the code. In addition, since we're defining macros around every function we can have the macro also convert from ptr to &(ptr) for us, avoiding this potential error: So the primary application usage would be via a set of macros: VIR_ALLOC(ptr) VIR_ALLOC_N(ptr, count) VIR_REALLOC(ptr) VIR_REALLOC_N(ptr, count) VIR_FREE(ptr) These call through to the underlying APIs: int virAlloc(void *, size_t bytes) int virAllocN(void *, size_t bytes, size_t count) int virRealloc(void *, size_t bytes) int virReallocN(void *, size_t bytes, size_t count) int virFree(void *) Note that although we're passing a pointer to a pointer into all these, the first param is still just 'void *' and not 'void **'. This is because 'void *' is defined to hold any type of pointer, and using 'void **' causes gcc to complain bitterly about strict aliasing violations. Internally the impls of these methods will safely cast to 'void **' when deferencing the pointer. All 4 of Alloc/Realloc functions will have __warn_unused_result__ annotation so the caller is forced to check the return value for failure, validated at compile time generating a warning (or fatal compile error with -Werror). So to wire up the macros to the APIs: #define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr))) #define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count)) #define VIR_REALLOC(ptr) virRealloc(&(ptr), sizeof(*(ptr))) #define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count)) #define VIR_FREE(ptr) virFree(&(ptr)) In particular notice how we're using the compiler to decide how many bytes to allocate for each variable here, thus guarenteeing the correct size. Finally here's an illustration of how these APis would be used in practice. - Allocating a new variable: virDomainPtr domain; if (VIR_ALLOC(domain) < 0) { __virRaiseError(VIR_ERROR_NO_MEMORY) return NULL; } Looking at the macro, this pass '&domain' into virAlloc() which initalizes it with address of memory sizeof(*domain) bytes long. - Allocating an array of domains virDomainPtr domains; int ndomains = 10; if (VIR_ALLOC_N(domains, ndomains) < 0) { __virRaiseError(VIR_ERROR_NO_MEMORY) return NULL; } - Allocating an array of domain pointers virDomainPtr *domains; int ndomains = 10; if (VIR_ALLOC_N(domains, ndomains) < 0) { __virRaiseError(VIR_ERROR_NO_MEMORY) return NULL; } - Re-allocate the array of domains to be longer ndomains = 20 if (VIR_REALLOC_N(domains, ndomains) < 0) { __virRaiseError(VIR_ERROR_NO_MEMORY) return NULL; } - Free the domain VIR_FREE(domain); The attached patch implements these ideas in a memory.h and memory.c file and updates 'hash.c' to illustrate their usage. If we were to replace every single instance of malloc/calloc/free/realloc with these calls the automated builds which run with -Werror would quickly tell us of any places where we forget to check memory allocations for failure, and we'd make code like from capabilities.c: if ((guest->arch.defaultInfo.machines = calloc(nmachines, sizeof(*guest->arch.defaultInfo.machines))) == NULL) return -1; char **migrateTrans; if ((migrateTrans = realloc(caps->host.migrateTrans, sizeof(*migrateTrans) * (caps->host.nmigrateTrans+1))) == NULL) return -1; caps->host.migrateTrans = migrateTrans; Much less ugly: if (VIR_ALLOC_N(guest->arch.defaultInfo.machines, nmachines) < 0) return -1; if (VIR_REALLOC(migrateTrans, caps->host.nmigrateTrans+1) < 0) return -1; So we've addressed all 7 common error scenarios with malloc/free/realloc/caller and produced easier to read code too. The cleaner realloc() API is particularly appealing. This does all seem a little bit too good to be true - I'm wondering why other apps/libraries don't use this style of allocation functions already ? Perhaps someone can find nasty flaws in this approach, but hopefuly not... hash.c | 113 ++++++++++++++++++++++-------------------------- memory.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ memory.h | 95 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 294 insertions(+), 60 deletions(-) Regards, Daniel. Index: memory.h =================================================================== RCS file: memory.h diff -N memory.h --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ memory.h 27 Apr 2008 19:04:28 -0000 @@ -0,0 +1,95 @@ +/* + * memory.c: safer memory allocation + * + * Copyright (C) 2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + + +#ifndef __VIR_MEMORY_H_ +#define __VIR_MEMORY_H_ + +#include "internal.h" + +/* Don't call these directly - use the macros below */ +int virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK; +int virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK; +int virRealloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK; +int virReallocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK; +void virFree(void *ptrptr); + + +/** + * VIR_ALLOC: + * @ptr: pointer to hold address of allocated memory + * + * Allocate sizeof(*ptr) bytes of memory and store + * the address of allocated memory in 'ptr'. Fill the + * newly allocated memory with zeros. + * + * Returns -1 on failure, 0 on success + */ +#define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr))) + +/** + * VIR_ALLOC_N: + * @ptr: pointer to hold address of allocated memory + * @count: number of elements to allocate + * + * Allocate an array of 'count' elements, each sizeof(*ptr) + * bytes long and store the address of allocated memory in + * 'ptr'. Fill the newly allocated memory with zeros. + * + * Returns -1 on failure, 0 on success + */ +#define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count)) + +/** + * VIR_REALLOC: + * @ptr: pointer to hold address of allocated memory + * + * Re-allocate to sizeof(*ptr) bytes of memory and store + * the address of allocated memory in 'ptr'. Fill the + * newly allocated memory with zeros + * + * Returns -1 on failure, 0 on success + */ +#define VIR_REALLOC(ptr) virRealloc(&(ptr), sizeof(*(ptr))) + +/** + * VIR_REALLOC_N: + * @ptr: pointer to hold address of allocated memory + * @count: number of elements to allocate + * + * Re-allocate an array of 'count' elements, each sizeof(*ptr) + * bytes long and store the address of allocated memory in + * 'ptr'. Fill the newly allocated memory with zeros + * + * Returns -1 on failure, 0 on success + */ +#define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count)) + +/** + * VIR_FREE: + * @ptr: pointer holding address to be freed + * + * Free the memory stored in 'ptr' and update to point + * to NULL. + */ +#define VIR_FREE(ptr) virFree(&(ptr)) + +#endif /* __VIR_MEMORY_H_ */ Index: memory.c =================================================================== RCS file: memory.c diff -N memory.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ memory.c 27 Apr 2008 19:04:28 -0000 @@ -0,0 +1,146 @@ +/* + * memory.c: safer memory allocation + * + * Copyright (C) 2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <stdlib.h> + +#include "memory.h" + +/** + * virAlloc: + * @ptrptr: pointer to pointer for address of allocated memory + * @size: number of bytes to allocate + * + * Allocate 'size' bytes of memory. Return the address of the + * allocated memory in 'ptrptr'. The newly allocated memory is + * filled with zeros. + * + * Returns -1 on failure to allocate, zero on success + */ +int virAlloc(void *ptrptr, size_t size) +{ + if (size == 0) { + *(void **)ptrptr = NULL; + return 0; + } + + *(void **)ptrptr = calloc(1, size); + if (*(void **)ptrptr == NULL) + return -1; + return 0; +} + +/** + * virAllocN: + * @ptrptr: pointer to pointer for address of allocated memory + * @size: number of bytes to allocate + * @count: number of elements to allocate + * + * Allocate an array of memory 'count' elements long, + * each with 'size' bytes. Return the address of the + * allocated memory in 'ptrptr'. The newly allocated + * memory is filled with zeros. + * + * Returns -1 on failure to allocate, zero on success + */ +int virAllocN(void *ptrptr, size_t size, size_t count) +{ + if (size == 0 || count == 0) { + *(void **)ptrptr = NULL; + return 0; + } + + *(void**)ptrptr = calloc(count, size); + if (*(void**)ptrptr == NULL) + return -1; + return 0; +} + +/** + * virRealloc: + * @ptrptr: pointer to pointer for address of allocated memory + * @size: number of bytes to allocate + * + * Resize the block of memory in 'ptrptr' to be 'size' bytes + * in length. Update 'ptrptr' with the address of the newly + * allocated memory. On failure, 'ptrptr' is not changed and + * still points to the original memory block. The newly + * allocated memory is filled with zeros. + * + * Returns -1 on failure to allocate, zero on success + */ +int virRealloc(void *ptrptr, size_t size) +{ + void *tmp; + if (size == 0) { + free(*(void **)ptrptr); + *(void **)ptrptr = NULL; + return 0; + } + + tmp = realloc(*(void**)ptrptr, size); + if (!tmp) + return -1; + *(void**)ptrptr = tmp; + return 0; +} + +/** + * virReallocN: + * @ptrptr: pointer to pointer for address of allocated memory + * @size: number of bytes to allocate + * @count: number of elements in array + * + * Resize the block of memory in 'ptrptr' to be an array of + * 'count' elements, each 'size' bytes in length. Update 'ptrptr' + * with the address of the newly allocated memory. On failure, + * 'ptrptr' is not changed and still points to the original memory + * block. The newly allocated memory is filled with zeros. + * + * Returns -1 on failure to allocate, zero on success + */ +int virReallocN(void *ptrptr, size_t size, size_t count) +{ + void *tmp; + if (size == 0 || count == 0) { + free(*(void **)ptrptr); + *(void **)ptrptr = NULL; + return 0; + } + tmp = realloc(*(void**)ptrptr, size * count); + if (!tmp) + return -1; + *(void**)ptrptr = tmp; + return 0; +} + +/** + * virFree: + * @ptrptr: pointer to pointer for address of memory to be freed + * + * Release the chunk of memory in the pointer pointed to by + * the 'ptrptr' variable. After release, 'ptrptr' will be + * updated to point to NULL. + */ +void virFree(void *ptrptr) +{ + free(*(void**)ptrptr); + *(void**)ptrptr = NULL; +} Index: hash.c =================================================================== RCS file: /data/cvs/libvirt/src/hash.c,v retrieving revision 1.37 diff -u -p -r1.37 hash.c --- hash.c 18 Apr 2008 08:33:23 -0000 1.37 +++ hash.c 27 Apr 2008 19:04:28 -0000 @@ -25,6 +25,7 @@ #include <libxml/threads.h> #include "internal.h" #include "hash.h" +#include "memory.h" #define MAX_HASH_LEN 8 @@ -85,22 +86,22 @@ virHashComputeKey(virHashTablePtr table, virHashTablePtr virHashCreate(int size) { - virHashTablePtr table; + virHashTablePtr table = NULL; if (size <= 0) size = 256; - table = malloc(sizeof(*table)); - if (table) { - table->size = size; - table->nbElems = 0; - table->table = calloc(1, size * sizeof(*(table->table))); - if (table->table) { - return (table); - } - free(table); + if (VIR_ALLOC(table) < 0) + return NULL; + + table->size = size; + table->nbElems = 0; + if (VIR_ALLOC_N(table->table, size) < 0) { + VIR_FREE(table); + return NULL; } - return (NULL); + + return table; } /** @@ -136,8 +137,7 @@ virHashGrow(virHashTablePtr table, int s if (oldtable == NULL) return (-1); - table->table = calloc(1, size * sizeof(*(table->table))); - if (table->table == NULL) { + if (VIR_ALLOC_N(table->table, size) < 0) { table->table = oldtable; return (-1); } @@ -170,7 +170,7 @@ virHashGrow(virHashTablePtr table, int s if (table->table[key].valid == 0) { memcpy(&(table->table[key]), iter, sizeof(virHashEntry)); table->table[key].next = NULL; - free(iter); + VIR_FREE(iter); } else { iter->next = table->table[key].next; table->table[key].next = iter; @@ -184,7 +184,7 @@ virHashGrow(virHashTablePtr table, int s } } - free(oldtable); + VIR_FREE(oldtable); #ifdef DEBUG_GROW xmlGenericError(xmlGenericErrorContext, @@ -225,19 +225,19 @@ virHashFree(virHashTablePtr table, virHa next = iter->next; if ((f != NULL) && (iter->payload != NULL)) f(iter->payload, iter->name); - free(iter->name); + VIR_FREE(iter->name); iter->payload = NULL; if (!inside_table) - free(iter); + VIR_FREE(iter); nbElems--; inside_table = 0; iter = next; } inside_table = 0; } - free(table->table); + VIR_FREE(table->table); } - free(table); + VIR_FREE(table); } /** @@ -281,8 +281,7 @@ virHashAddEntry(virHashTablePtr table, c if (insert == NULL) { entry = &(table->table[key]); } else { - entry = malloc(sizeof(*entry)); - if (entry == NULL) + if (VIR_ALLOC(entry) < 0) return (-1); } @@ -354,8 +353,7 @@ virHashUpdateEntry(virHashTablePtr table if (insert == NULL) { entry = &(table->table[key]); } else { - entry = malloc(sizeof(*entry)); - if (entry == NULL) + if (VIR_ALLOC(entry) < 0) return (-1); } @@ -451,10 +449,10 @@ virHashRemoveEntry(virHashTablePtr table if ((f != NULL) && (entry->payload != NULL)) f(entry->payload, entry->name); entry->payload = NULL; - free(entry->name); + VIR_FREE(entry->name); if (prev) { prev->next = entry->next; - free(entry); + VIR_FREE(entry); } else { if (entry->next == NULL) { entry->valid = 0; @@ -462,7 +460,7 @@ virHashRemoveEntry(virHashTablePtr table entry = entry->next; memcpy(&(table->table[key]), entry, sizeof(virHashEntry)); - free(entry); + VIR_FREE(entry); } } table->nbElems--; @@ -535,11 +533,11 @@ int virHashRemoveSet(virHashTablePtr tab if (iter(entry->payload, entry->name, data)) { count++; f(entry->payload, entry->name); - free(entry->name); + VIR_FREE(entry->name); table->nbElems--; if (prev) { prev->next = entry->next; - free(entry); + VIR_FREE(entry); entry = prev; } else { if (entry->next == NULL) { @@ -549,7 +547,7 @@ int virHashRemoveSet(virHashTablePtr tab entry = entry->next; memcpy(&(table->table[i]), entry, sizeof(virHashEntry)); - free(entry); + VIR_FREE(entry); entry = &(table->table[i]); continue; } @@ -689,8 +687,7 @@ virConnectPtr virGetConnect(void) { virConnectPtr ret; - ret = calloc(1, sizeof(*ret)); - if (ret == NULL) { + if (VIR_ALLOC(ret) < 0) { virHashError(NULL, VIR_ERR_NO_MEMORY, _("allocating connection")); goto failed; } @@ -729,7 +726,7 @@ failed: virHashFree(ret->storageVols, (virHashDeallocator) virStorageVolFreeName); pthread_mutex_destroy(&ret->lock); - free(ret); + VIR_FREE(ret); } return(NULL); } @@ -759,11 +756,11 @@ virReleaseConnect(virConnectPtr conn) { if (__lastErr.conn == conn) __lastErr.conn = NULL; - free(conn->name); + VIR_FREE(conn->name); pthread_mutex_unlock(&conn->lock); pthread_mutex_destroy(&conn->lock); - free(conn); + VIR_FREE(conn); } /** @@ -824,8 +821,7 @@ __virGetDomain(virConnectPtr conn, const ret = (virDomainPtr) virHashLookup(conn->domains, name); /* TODO check the UUID */ if (ret == NULL) { - ret = (virDomainPtr) calloc(1, sizeof(*ret)); - if (ret == NULL) { + if (VIR_ALLOC(ret) < 0) { virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain")); goto error; } @@ -854,8 +850,8 @@ __virGetDomain(virConnectPtr conn, const error: pthread_mutex_unlock(&conn->lock); if (ret != NULL) { - free(ret->name ); - free(ret); + VIR_FREE(ret->name); + VIR_FREE(ret); } return(NULL); } @@ -888,8 +884,8 @@ virReleaseDomain(virDomainPtr domain) { __lastErr.dom = NULL; domain->magic = -1; domain->id = -1; - free(domain->name); - free(domain); + VIR_FREE(domain->name); + VIR_FREE(domain); DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs); conn->refs--; @@ -962,8 +958,7 @@ __virGetNetwork(virConnectPtr conn, cons ret = (virNetworkPtr) virHashLookup(conn->networks, name); /* TODO check the UUID */ if (ret == NULL) { - ret = (virNetworkPtr) calloc(1, sizeof(*ret)); - if (ret == NULL) { + if (VIR_ALLOC(ret) < 0) { virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network")); goto error; } @@ -991,8 +986,8 @@ __virGetNetwork(virConnectPtr conn, cons error: pthread_mutex_unlock(&conn->lock); if (ret != NULL) { - free(ret->name ); - free(ret); + VIR_FREE(ret->name); + VIR_FREE(ret); } return(NULL); } @@ -1025,8 +1020,8 @@ virReleaseNetwork(virNetworkPtr network) __lastErr.net = NULL; network->magic = -1; - free(network->name); - free(network); + VIR_FREE(network->name); + VIR_FREE(network); DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs); conn->refs--; @@ -1100,8 +1095,7 @@ __virGetStoragePool(virConnectPtr conn, ret = (virStoragePoolPtr) virHashLookup(conn->storagePools, name); /* TODO check the UUID */ if (ret == NULL) { - ret = (virStoragePoolPtr) calloc(1, sizeof(*ret)); - if (ret == NULL) { + if (VIR_ALLOC(ret) < 0) { virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating storage pool")); goto error; } @@ -1129,8 +1123,8 @@ __virGetStoragePool(virConnectPtr conn, error: pthread_mutex_unlock(&conn->lock); if (ret != NULL) { - free(ret->name); - free(ret); + VIR_FREE(ret->name); + VIR_FREE(ret); } return(NULL); } @@ -1159,8 +1153,8 @@ virReleaseStoragePool(virStoragePoolPtr _("pool missing from connection hash table")); pool->magic = -1; - free(pool->name); - free(pool); + VIR_FREE(pool->name); + VIR_FREE(pool); DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs); conn->refs--; @@ -1232,8 +1226,7 @@ __virGetStorageVol(virConnectPtr conn, c ret = (virStorageVolPtr) virHashLookup(conn->storageVols, key); if (ret == NULL) { - ret = (virStorageVolPtr) calloc(1, sizeof(*ret)); - if (ret == NULL) { + if (VIR_ALLOC(ret) < 0) { virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating storage vol")); goto error; } @@ -1266,9 +1259,9 @@ __virGetStorageVol(virConnectPtr conn, c error: pthread_mutex_unlock(&conn->lock); if (ret != NULL) { - free(ret->name); - free(ret->pool); - free(ret); + VIR_FREE(ret->name); + VIR_FREE(ret->pool); + VIR_FREE(ret); } return(NULL); } @@ -1297,9 +1290,9 @@ virReleaseStorageVol(virStorageVolPtr vo _("vol missing from connection hash table")); vol->magic = -1; - free(vol->name); - free(vol->pool); - free(vol); + VIR_FREE(vol->name); + VIR_FREE(vol->pool); + VIR_FREE(vol); DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs); conn->refs--; -- |: 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 :|

6 18

[Libvir] PATCH: Make the virBuffer API harder to misuse
by Daniel P. Berrange 29 Apr '08

29 Apr '08

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. - 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. - All virBuffer objects must now be declared on the stack and initialized with the static VIR_BUFFER_INITIALIZER, instead of the virBufferNew() allocator Rationale: avoids the need to check virBufferNew for allocation failure, and avoids a potential memory leak of the virBuffer object itself. - The buffer content can only be accessed via the public API virBufferContentAndReset(). This returns the internal char * content, and resets the state of the buffer. Rationale: the caller can only obtain the internal content if the buffer is in a known good state. On error they'll be given back a NULL, rather than potentially incomplete / malformed data. - The virBufferAdd/VSprintf/etc routines for appending to the buffer are all void and set the internal error flag upon failure and immediately free the internal content. Rationale: the caller will never see a potentially incomplete buffer, and avoids the potential of leaking the memory for the internal buffer in cleanup paths. So in using the APIs, there are now only a handful of very low impact errors the caller can make: - Forget to initialize the buffer with VIR_BUFFER_INITIALIZER. The compiler will warn about this, so it is harmless. - Forget to free() the content from virBufferContentAndReset. This data is typically propagated to the caller's caller for free()ing anyway, so not an issue. - Forget to check the error flag before asking for the internal content. If the error flag is set the internal content will have been free'd and they'll be given back NULL. So the caller will not see incomplete data. So the worst case behaviour is a leak of the buffer content, or failing to raise a libvirt error when the buffer has an error. The general style of usage now looks like char * somefunction(...) { virBuffer buf = VIR_BUFFER_INITIALIZER; ... virBufferAdd(&buf, "<domain>\n"); virBufferVSprint(&buf, " <memory>%d</memory>\n", memory); ... virBufferAdd(&buf, "</domain>\n"); .... if (virBufferError(&buf)) { __virRaiseError(...); return NULL; } return virBufferContentAndReset(&buf); } In fixing up all the uses of virBuffer routines in existing driver code I encountered every error condition mentioned here. Most of these would have resulted in either memory leaks, or passing a malformed XML document to another routine, so with the additional code clarity gained by removing the gotos this is very useful cleanup. src/buf.c | 241 ++++++++++++++++------------- src/buf.h | 50 ++++-- src/capabilities.c | 187 ++++++++-------------- src/conf.c | 77 ++++----- src/libvirt_sym.version | 2 src/lxc_conf.c | 104 +++--------- src/qemu_conf.c | 391 ++++++++++++++++++------------------------------ src/qparams.c | 15 + src/storage_conf.c | 227 ++++++++++----------------- src/test.c | 74 ++++----- src/virsh.c | 158 ++++++++----------- src/xend_internal.c | 126 ++++++--------- src/xm_internal.c | 184 ++++++++++------------ src/xml.c | 101 +++++------- src/xmlrpc.c | 22 +- src/xmlrpc.h | 5 tests/xmlrpctest.c | 81 ++++----- 17 files changed, 898 insertions(+), 1147 deletions(-) Regards, Daniel. Index: src/buf.c =================================================================== RCS file: /data/cvs/libvirt/src/buf.c,v retrieving revision 1.15 diff -u -p -r1.15 buf.c --- src/buf.c 10 Apr 2008 16:54:54 -0000 1.15 +++ src/buf.c 26 Apr 2008 16:24:38 -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 increase + the public __SIZEOF_VIR_BUFFER constant */ +struct _virBuffer { + unsigned int size; + unsigned int use; + unsigned int error; + char *content; +}; + +/** + * virBufferFail + * @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) +{ + free(buf->content); + buf->content = NULL; + buf->size = 0; + buf->use = 0; + buf->error = 1; +} + /** * virBufferGrow: * @buf: the buffer @@ -27,7 +56,7 @@ * * Grow the available space of a buffer to at least @len bytes. * - * Returns the new available space or -1 in case of error + * Returns zero on success or -1 on error */ static int virBufferGrow(virBufferPtr buf, unsigned int len) @@ -35,18 +64,22 @@ virBufferGrow(virBufferPtr buf, unsigned int size; char *newbuf; - if (buf == NULL) - return (-1); - if (len + buf->use < buf->size) - return (0); + if (buf->error) + return -1; + + if ((len + buf->use) < buf->size) + return 0; size = buf->use + len + 1000; newbuf = realloc(buf->content, size); - if (newbuf == NULL) return -1; + if (newbuf == NULL) { + virBufferNoMemory(buf); + return -1; + } buf->content = newbuf; buf->size = size; - return (buf->size - buf->use); + return 0; } /** @@ -58,33 +91,29 @@ virBufferGrow(virBufferPtr buf, unsigned * Add a string range to an XML buffer. if len == -1, the length of * str is recomputed to the full string. * - * Returns 0 successful, -1 in case of internal or API error. */ -int -__virBufferAdd(virBufferPtr buf, const char *str, int len) +void +__virBufferAdd(const virBufferPtr buf, const char *str, int len) { unsigned int needSize; - if ((str == NULL) || (buf == NULL)) { - return -1; - } - if (len == 0) - return 0; + if ((str == NULL) || (buf == NULL) || (len == 0)) + return; + + if (buf->error) + return; if (len < 0) len = strlen(str); needSize = buf->use + len + 2; - if (needSize > buf->size) { - if (!virBufferGrow(buf, needSize - buf->use)) { - return (-1); - } - } + if (needSize > buf->size && + virBufferGrow(buf, needSize - buf->use) < 0) + return; memcpy (&buf->content[buf->use], str, len); buf->use += len; - buf->content[buf->use] = 0; - return (0); + buf->content[buf->use] = '\0'; } /** @@ -94,89 +123,85 @@ __virBufferAdd(virBufferPtr buf, const c * * Add a single character 'c' to a buffer. * - * Returns 0 if successful, -1 in the case of error. */ -int +void __virBufferAddChar (virBufferPtr buf, char c) { unsigned int needSize; if (buf == NULL) - return -1; + return; + + if (buf->error) + return; needSize = buf->use + 2; - if (needSize > buf->size) - if (!virBufferGrow (buf, needSize - buf->use)) - return -1; + if (needSize > buf->size && + virBufferGrow (buf, needSize - buf->use) < 0) + return; buf->content[buf->use++] = c; - buf->content[buf->use] = 0; - - return 0; + buf->content[buf->use] = '\0'; } /** - * virBufferNew: - * @size: creation size in bytes + * virBufferContentAndReset: + * @buf: Buffer * - * Creates a new buffer + * Get the content from the buffer and free (only) the buffer structure. + * The caller owns the returned string & should free it when no longer + * required. The buffer object is reset to its initial state. * - * Returns a pointer to the buffer or NULL in case of error + * Returns the buffer content or NULL in case of error. */ -virBufferPtr -virBufferNew(unsigned int size) +char * +__virBufferContentAndReset(const virBufferPtr buf) { - virBufferPtr buf; + char *str; + if (buf == NULL) + return NULL; - if (!(buf = malloc(sizeof(*buf)))) return NULL; - if (size && (buf->content = malloc(size))==NULL) { - free(buf); + if (buf->error) { + memset(buf, 0, sizeof(*buf)); return NULL; } - buf->size = size; - buf->use = 0; - return buf; + str = buf->content; + memset(buf, 0, sizeof(*buf)); + return str; } /** - * virBufferFree: - * @buf: the buffer to deallocate + * virBufferError: + * @buf: the buffer * - * Free the set of resources used by a buffer. + * Check to see if the buffer is in an error state due + * to failed memory allocation + * + * Return true if in error, 0 if normal */ - -void -virBufferFree(virBufferPtr buf) +int +__virBufferError(const virBufferPtr buf) { - if (buf) { - free(buf->content); - free(buf); - } + if (buf == NULL) + return 1; + + return buf->error; } /** - * virBufferContentAndFree: - * @buf: Buffer + * virBufferUse: + * @buf: the usage of the string in the buffer * - * Get the content from the buffer and free (only) the buffer structure. - * - * Returns the buffer content or NULL in case of error. + * Return the string usage in bytes */ -char * -virBufferContentAndFree (virBufferPtr buf) +unsigned int +virBufferUse(const virBufferPtr buf) { - char *content; - if (buf == NULL) - return(NULL); - - content = buf->content; - if (content != NULL) - content[buf->use] = 0; + return 0; - free (buf); - return(content); + return buf->use; } /** @@ -186,22 +211,22 @@ virBufferContentAndFree (virBufferPtr bu * @...: the variable list of arguments * * Do a formatted print to an XML buffer. - * - * Returns 0 successful, -1 in case of internal or API error. */ -int -__virBufferVSprintf(virBufferPtr buf, const char *format, ...) +void +__virBufferVSprintf(const virBufferPtr buf, const char *format, ...) { int size, count, grow_size; va_list locarg, argptr; - if ((format == NULL) || (buf == NULL)) { - return (-1); - } + if ((format == NULL) || (buf == NULL)) + return; + + if (buf->error) + return; if (buf->size == 0 && virBufferGrow(buf, 100) < 0) - return -1; + return; size = buf->size - buf->use - 1; va_start(argptr, format); @@ -210,17 +235,17 @@ __virBufferVSprintf(virBufferPtr buf, co locarg)) < 0) || (count >= size - 1)) { buf->content[buf->use] = 0; va_end(locarg); + grow_size = (count > 1000) ? count : 1000; - if (virBufferGrow(buf, grow_size) < 0) { - return (-1); - } + if (virBufferGrow(buf, grow_size) < 0) + return; + size = buf->size - buf->use - 1; va_copy(locarg, argptr); } va_end(locarg); buf->use += count; - buf->content[buf->use] = 0; - return (0); + buf->content[buf->use] = '\0'; } /** @@ -231,25 +256,27 @@ __virBufferVSprintf(virBufferPtr buf, co * * Do a formatted print with a single string to an XML buffer. The string * is escaped to avoid generating a not well-formed XML instance. - * - * Returns 0 successful, -1 in case of internal or API error. */ -int -virBufferEscapeString(virBufferPtr buf, const char *format, const char *str) +void +virBufferEscapeString(const virBufferPtr buf, const char *format, const char *str) { int size, count, len, grow_size; char *escaped, *out; const char *cur; - if ((format == NULL) || (buf == NULL) || (str == NULL)) { - return (-1); - } + if ((format == NULL) || (buf == NULL) || (str == NULL)) + return; + + if (buf->error) + return; len = strlen(str); escaped = malloc(5 * len + 1); if (escaped == NULL) { - return (-1); + virBufferNoMemory(buf); + return; } + cur = str; out = escaped; while (*cur != 0) { @@ -290,14 +317,13 @@ virBufferEscapeString(virBufferPtr buf, grow_size = (count > 1000) ? count : 1000; if (virBufferGrow(buf, grow_size) < 0) { free(escaped); - return (-1); + return; } size = buf->size - buf->use - 1; } buf->use += count; - buf->content[buf->use] = 0; + buf->content[buf->use] = '\0'; free(escaped); - return (0); } /** @@ -308,10 +334,8 @@ virBufferEscapeString(virBufferPtr buf, * Append the string to the buffer. The string will be URI-encoded * during the append (ie any non alpha-numeric characters are replaced * with '%xx' hex sequences). - * - * Returns 0 successful, -1 in case of internal or API error. */ -int +void virBufferURIEncodeString (virBufferPtr buf, const char *str) { int grow_size = 0; @@ -319,6 +343,12 @@ virBufferURIEncodeString (virBufferPtr b unsigned char uc; const char *hex = "0123456789abcdef"; + if ((buf == NULL) || (str == NULL)) + return; + + if (buf->error) + return; + for (p = str; *p; ++p) { /* This may not work on EBCDIC. */ if ((*p >= 'a' && *p <= 'z') || @@ -329,8 +359,8 @@ virBufferURIEncodeString (virBufferPtr b grow_size += 3; /* %ab */ } - if (virBufferGrow (buf, grow_size) == -1) - return -1; + if (virBufferGrow (buf, grow_size) < 0) + return; for (p = str; *p; ++p) { /* This may not work on EBCDIC. */ @@ -347,7 +377,6 @@ virBufferURIEncodeString (virBufferPtr b } buf->content[buf->use] = '\0'; - return 0; } /** @@ -356,15 +385,16 @@ virBufferURIEncodeString (virBufferPtr b * @...: the variable list of strings, the last argument must be NULL * * Concatenate strings to an XML buffer. - * - * Returns 0 successful, -1 in case of internal or API error. */ -int +void virBufferStrcat(virBufferPtr buf, ...) { va_list ap; char *str; + if (buf->error) + return; + va_start(ap, buf); while ((str = va_arg(ap, char *)) != NULL) { @@ -372,13 +402,12 @@ virBufferStrcat(virBufferPtr buf, ...) unsigned int needSize = buf->use + len + 2; if (needSize > buf->size) { - if (!virBufferGrow(buf, needSize - buf->use)) - return -1; + if (virBufferGrow(buf, needSize - buf->use) < 0) + return; } memcpy(&buf->content[buf->use], str, len); buf->use += len; buf->content[buf->use] = 0; } va_end(ap); - return 0; } Index: src/buf.h =================================================================== RCS file: /data/cvs/libvirt/src/buf.h,v retrieving revision 1.5 diff -u -p -r1.5 buf.h --- src/buf.h 20 Feb 2008 15:29:13 -0000 1.5 +++ src/buf.h 26 Apr 2008 16:24:38 -0000 @@ -20,22 +20,45 @@ */ typedef struct _virBuffer virBuffer; typedef virBuffer *virBufferPtr; + +#ifndef __VIR_BUFFER_C__ +/* This constant must match size of the actual struct + in the buf.c file */ +#if __WORDSIZE == 64 +#define __SIZEOF_VIR_BUFFER 32 +#define VIR_BUFFER_INITIALIZER { { 0, 0, 0, 0, \ + 0, 0, 0, 0, \ + 0, 0, 0, 0, \ + 0, 0, 0, 0, \ + 0, 0, 0, 0, \ + 0, 0, 0, 0, \ + 0, 0, 0, 0, \ + 0, 0, 0, 0 \ + } } +#else +#define __SIZEOF_VIR_BUFFER 16 +#define VIR_BUFFER_INITIALIZER { { 0, 0, 0, 0, \ + 0, 0, 0, 0, \ + 0, 0, 0, 0, \ + 0, 0, 0, 0 \ + } } +#endif 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 */ }; -virBufferPtr virBufferNew(unsigned int size); -void virBufferFree(virBufferPtr buf); -char *virBufferContentAndFree(virBufferPtr buf); -int __virBufferAdd(virBufferPtr buf, const char *str, int len); -int __virBufferAddChar(virBufferPtr buf, char c); -int __virBufferVSprintf(virBufferPtr buf, const char *format, ...) +#endif + +char *__virBufferContentAndReset(const virBufferPtr buf); +int __virBufferError(const virBufferPtr buf); +unsigned int virBufferUse(const virBufferPtr buf); +void __virBufferAdd(const virBufferPtr buf, const char *str, int len); +void __virBufferAddChar(const virBufferPtr buf, char c); +void __virBufferVSprintf(const virBufferPtr buf, const char *format, ...) ATTRIBUTE_FORMAT(printf, 2, 3); -int virBufferStrcat(virBufferPtr buf, ...); -int virBufferEscapeString(virBufferPtr buf, const char *format, const char *str); -int virBufferURIEncodeString (virBufferPtr buf, const char *str); +void virBufferStrcat(const virBufferPtr buf, ...); +void virBufferEscapeString(const virBufferPtr buf, const char *format, const char *str); +void virBufferURIEncodeString (const virBufferPtr buf, const char *str); #define virBufferAddLit(buf_, literal_string_) \ __virBufferAdd (buf_, "" literal_string_ "", sizeof literal_string_ - 1) @@ -44,4 +67,7 @@ int virBufferURIEncodeString (virBufferP #define virBufferAddChar(b,c) __virBufferAddChar((b),(c)) #define virBufferVSprintf(b,f,...) __virBufferVSprintf((b),(f), __VA_ARGS__) +#define virBufferContentAndReset(b) __virBufferContentAndReset((b)) +#define virBufferError(b) __virBufferError((b)) + #endif /* __VIR_BUFFER_H__ */ Index: src/capabilities.c =================================================================== RCS file: /data/cvs/libvirt/src/capabilities.c,v retrieving revision 1.6 diff -u -p -r1.6 capabilities.c --- src/capabilities.c 10 Apr 2008 16:53:29 -0000 1.6 +++ src/capabilities.c 26 Apr 2008 16:24:39 -0000 @@ -521,172 +521,127 @@ virCapabilitiesDefaultGuestEmulator(virC char * virCapabilitiesFormatXML(virCapsPtr caps) { - virBuffer xml = { NULL, 0, 0 }; + virBuffer xml = VIR_BUFFER_INITIALIZER; int i, j, k; - if (virBufferAddLit(&xml, "<capabilities>\n\n") < 0) - goto no_memory; - if (virBufferAddLit(&xml, " <host>\n") < 0) - goto no_memory; - if (virBufferAddLit(&xml, " <cpu>\n") < 0) - goto no_memory; - if (virBufferVSprintf(&xml, " <arch>%s</arch>\n", - caps->host.arch) < 0) - goto no_memory; + virBufferAddLit(&xml, "<capabilities>\n\n"); + virBufferAddLit(&xml, " <host>\n"); + virBufferAddLit(&xml, " <cpu>\n"); + virBufferVSprintf(&xml, " <arch>%s</arch>\n", + caps->host.arch); if (caps->host.nfeatures) { - if (virBufferAddLit(&xml, " <features>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " <features>\n"); for (i = 0 ; i < caps->host.nfeatures ; i++) { - if (virBufferVSprintf(&xml, " <%s/>\n", - caps->host.features[i]) <0) - goto no_memory; + virBufferVSprintf(&xml, " <%s/>\n", + caps->host.features[i]); } - if (virBufferAddLit(&xml, " </features>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " </features>\n"); } - if (virBufferAddLit(&xml, " </cpu>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " </cpu>\n"); if (caps->host.offlineMigrate) { - if (virBufferAddLit(&xml, " <migration_features>\n") < 0) - goto no_memory; - if (caps->host.liveMigrate && - virBufferAddLit(&xml, " <live/>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " <migration_features>\n"); + if (caps->host.liveMigrate) + virBufferAddLit(&xml, " <live/>\n"); if (caps->host.nmigrateTrans) { - if (virBufferAddLit(&xml, " <uri_transports>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " <uri_transports>\n"); for (i = 0 ; i < caps->host.nmigrateTrans ; i++) { - if (virBufferVSprintf(&xml, " <uri_transport>%s</uri_transport>\n", - caps->host.migrateTrans[i]) < 0) - goto no_memory; + virBufferVSprintf(&xml, " <uri_transport>%s</uri_transport>\n", + caps->host.migrateTrans[i]); } - if (virBufferAddLit(&xml, " </uri_transports>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " </uri_transports>\n"); } - if (virBufferAddLit(&xml, " </migration_features>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " </migration_features>\n"); } if (caps->host.nnumaCell) { - if (virBufferAddLit(&xml, " <topology>\n") < 0) - goto no_memory; - if (virBufferVSprintf(&xml, " <cells num='%d'>\n", - caps->host.nnumaCell) < 0) - goto no_memory; + virBufferAddLit(&xml, " <topology>\n"); + virBufferVSprintf(&xml, " <cells num='%d'>\n", + caps->host.nnumaCell); for (i = 0 ; i < caps->host.nnumaCell ; i++) { - if (virBufferVSprintf(&xml, " <cell id='%d'>\n", - caps->host.numaCell[i]->num) < 0) - goto no_memory; - if (virBufferVSprintf(&xml, " <cpus num='%d'>\n", - caps->host.numaCell[i]->ncpus) < 0) - goto no_memory; + virBufferVSprintf(&xml, " <cell id='%d'>\n", + caps->host.numaCell[i]->num); + virBufferVSprintf(&xml, " <cpus num='%d'>\n", + caps->host.numaCell[i]->ncpus); for (j = 0 ; j < caps->host.numaCell[i]->ncpus ; j++) - if (virBufferVSprintf(&xml, " <cpu id='%d'/>\n", - caps->host.numaCell[i]->cpus[j]) < 0) - goto no_memory; - if (virBufferAddLit(&xml, " </cpus>\n") < 0) - goto no_memory; - if (virBufferAddLit(&xml, " </cell>\n") < 0) - goto no_memory; - } - if (virBufferAddLit(&xml, " </cells>\n") < 0) - goto no_memory; - if (virBufferAddLit(&xml, " </topology>\n") < 0) - goto no_memory; + virBufferVSprintf(&xml, " <cpu id='%d'/>\n", + caps->host.numaCell[i]->cpus[j]); + virBufferAddLit(&xml, " </cpus>\n"); + virBufferAddLit(&xml, " </cell>\n"); + } + virBufferAddLit(&xml, " </cells>\n"); + virBufferAddLit(&xml, " </topology>\n"); } - if (virBufferAddLit(&xml, " </host>\n\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " </host>\n\n"); for (i = 0 ; i < caps->nguests ; i++) { - if (virBufferAddLit(&xml, " <guest>\n") < 0) - goto no_memory; - if (virBufferVSprintf(&xml, " <os_type>%s</os_type>\n", - caps->guests[i]->ostype) < 0) - goto no_memory; - if (virBufferVSprintf(&xml, " <arch name='%s'>\n", - caps->guests[i]->arch.name) < 0) - goto no_memory; - if (virBufferVSprintf(&xml, " <wordsize>%d</wordsize>\n", - caps->guests[i]->arch.wordsize) < 0) - goto no_memory; - if (caps->guests[i]->arch.defaultInfo.emulator && + virBufferAddLit(&xml, " <guest>\n"); + virBufferVSprintf(&xml, " <os_type>%s</os_type>\n", + caps->guests[i]->ostype); + virBufferVSprintf(&xml, " <arch name='%s'>\n", + caps->guests[i]->arch.name); + virBufferVSprintf(&xml, " <wordsize>%d</wordsize>\n", + caps->guests[i]->arch.wordsize); + if (caps->guests[i]->arch.defaultInfo.emulator) virBufferVSprintf(&xml, " <emulator>%s</emulator>\n", - caps->guests[i]->arch.defaultInfo.emulator) < 0) - goto no_memory; - if (caps->guests[i]->arch.defaultInfo.loader && - virBufferVSprintf(&xml, " <loader>%s</loader>\n", - caps->guests[i]->arch.defaultInfo.loader) < 0) - goto no_memory; + caps->guests[i]->arch.defaultInfo.emulator); + if (caps->guests[i]->arch.defaultInfo.loader) + virBufferVSprintf(&xml, " <loader>%s</loader>\n", + caps->guests[i]->arch.defaultInfo.loader); for (j = 0 ; j < caps->guests[i]->arch.defaultInfo.nmachines ; j++) { - if (virBufferVSprintf(&xml, " <machine>%s</machine>\n", - caps->guests[i]->arch.defaultInfo.machines[j]) < 0) - goto no_memory; + virBufferVSprintf(&xml, " <machine>%s</machine>\n", + caps->guests[i]->arch.defaultInfo.machines[j]); } for (j = 0 ; j < caps->guests[i]->arch.ndomains ; j++) { - if (virBufferVSprintf(&xml, " <domain type='%s'>\n", - caps->guests[i]->arch.domains[j]->type) < 0) - goto no_memory; - if (caps->guests[i]->arch.domains[j]->info.emulator && + virBufferVSprintf(&xml, " <domain type='%s'>\n", + caps->guests[i]->arch.domains[j]->type); + if (caps->guests[i]->arch.domains[j]->info.emulator) virBufferVSprintf(&xml, " <emulator>%s</emulator>\n", - caps->guests[i]->arch.domains[j]->info.emulator) < 0) - goto no_memory; - if (caps->guests[i]->arch.domains[j]->info.loader && + caps->guests[i]->arch.domains[j]->info.emulator); + if (caps->guests[i]->arch.domains[j]->info.loader) virBufferVSprintf(&xml, " <loader>%s</loader>\n", - caps->guests[i]->arch.domains[j]->info.loader) < 0) - goto no_memory; + caps->guests[i]->arch.domains[j]->info.loader); for (k = 0 ; k < caps->guests[i]->arch.domains[j]->info.nmachines ; k++) { - if (virBufferVSprintf(&xml, " <machine>%s</machine>\n", - caps->guests[i]->arch.domains[j]->info.machines[k]) < 0) - goto no_memory; + virBufferVSprintf(&xml, " <machine>%s</machine>\n", + caps->guests[i]->arch.domains[j]->info.machines[k]); } - if (virBufferAddLit(&xml, " </domain>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " </domain>\n"); } - if (virBufferAddLit(&xml, " </arch>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " </arch>\n"); if (caps->guests[i]->nfeatures) { - if (virBufferAddLit(&xml, " <features>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " <features>\n"); for (j = 0 ; j < caps->guests[i]->nfeatures ; j++) { if (STREQ(caps->guests[i]->features[j]->name, "pae") || STREQ(caps->guests[i]->features[j]->name, "nonpae") || STREQ(caps->guests[i]->features[j]->name, "ia64_be")) { - if (virBufferVSprintf(&xml, " <%s/>\n", - caps->guests[i]->features[j]->name) < 0) - goto no_memory; + virBufferVSprintf(&xml, " <%s/>\n", + caps->guests[i]->features[j]->name); } else { - if (virBufferVSprintf(&xml, " <%s default='%s' toggle='%s'/>\n", - caps->guests[i]->features[j]->name, - caps->guests[i]->features[j]->defaultOn ? "on" : "off", - caps->guests[i]->features[j]->toggle ? "yes" : "no") < 0) - goto no_memory; + virBufferVSprintf(&xml, " <%s default='%s' toggle='%s'/>\n", + caps->guests[i]->features[j]->name, + caps->guests[i]->features[j]->defaultOn ? "on" : "off", + caps->guests[i]->features[j]->toggle ? "yes" : "no"); } } - if (virBufferAddLit(&xml, " </features>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " </features>\n"); } - - if (virBufferAddLit(&xml, " </guest>\n\n") < 0) - goto no_memory; + virBufferAddLit(&xml, " </guest>\n\n"); } - if (virBufferAddLit(&xml, "</capabilities>\n") < 0) - goto no_memory; + virBufferAddLit(&xml, "</capabilities>\n"); - return xml.content; + if (virBufferError(&xml)) + return NULL; - no_memory: - free(xml.content); - return NULL; + return virBufferContentAndReset(&xml); } Index: src/conf.c =================================================================== RCS file: /data/cvs/libvirt/src/conf.c,v retrieving revision 1.25 diff -u -p -r1.25 conf.c --- src/conf.c 10 Apr 2008 16:54:54 -0000 1.25 +++ src/conf.c 26 Apr 2008 16:24:39 -0000 @@ -877,43 +877,45 @@ __virConfSetValue (virConfPtr conf, int __virConfWriteFile(const char *filename, virConfPtr conf) { - virBufferPtr buf; + virBuffer buf = VIR_BUFFER_INITIALIZER; virConfEntryPtr cur; int ret; int fd; + char *content; + unsigned int use; if (conf == NULL) return(-1); - buf = virBufferNew(500); - if (buf == NULL) { - virConfError(NULL, VIR_ERR_NO_MEMORY, _("failed to allocate buffer"), 0); - return(-1); - } - cur = conf->entries; while (cur != NULL) { - virConfSaveEntry(buf, cur); + virConfSaveEntry(&buf, cur); cur = cur->next; } + if (virBufferError(&buf)) { + virConfError(NULL, VIR_ERR_NO_MEMORY, _("allocate buffer"), 0); + return -1; + } + fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR ); if (fd < 0) { virConfError(NULL, VIR_ERR_WRITE_FAILED, _("failed to open file"), 0); - ret = -1; - goto error; + free(virBufferContentAndReset(&buf)); + return -1; } - ret = safewrite(fd, buf->content, buf->use); + use = virBufferUse(&buf); + content = virBufferContentAndReset(&buf); + ret = safewrite(fd, content, use); + free(content); close(fd); - if (ret != (int) buf->use) { + if (ret != (int)use) { virConfError(NULL, VIR_ERR_WRITE_FAILED, _("failed to save content"), 0); - ret = -1; - goto error; + return -1; } -error: - virBufferFree(buf); - return(ret); + + return ret; } /** @@ -932,34 +934,35 @@ error: int __virConfWriteMem(char *memory, int *len, virConfPtr conf) { - virBufferPtr buf; + virBuffer buf = VIR_BUFFER_INITIALIZER; virConfEntryPtr cur; - int ret; + char *content; + unsigned int use; if ((memory == NULL) || (len == NULL) || (*len <= 0) || (conf == NULL)) return(-1); - buf = virBufferNew(500); - if (buf == NULL) { - virConfError(NULL, VIR_ERR_NO_MEMORY, _("failed to allocate buffer"), 0); - return(-1); - } - cur = conf->entries; while (cur != NULL) { - virConfSaveEntry(buf, cur); + virConfSaveEntry(&buf, cur); cur = cur->next; } - if ((int) buf->use >= *len) { - *len = buf->use; - ret = -1; - goto error; - } - memcpy(memory, buf->content, buf->use); - ret = buf->use; - *len = buf->use; -error: - virBufferFree(buf); - return(ret); + if (virBufferError(&buf)) { + virConfError(NULL, VIR_ERR_NO_MEMORY, _("allocate buffer"), 0); + return -1; + } + + use = virBufferUse(&buf); + content = virBufferContentAndReset(&buf); + + if ((int)use >= *len) { + *len = (int)use; + free(content); + return -1; + } + memcpy(memory, content, use); + free(content); + *len = use; + return use; } Index: src/libvirt_sym.version =================================================================== RCS file: /data/cvs/libvirt/src/libvirt_sym.version,v retrieving revision 1.38 diff -u -p -r1.38 libvirt_sym.version --- src/libvirt_sym.version 28 Feb 2008 17:06:32 -0000 1.38 +++ src/libvirt_sym.version 26 Apr 2008 16:24:39 -0000 @@ -184,6 +184,8 @@ __virBufferVSprintf; __virBufferAdd; __virBufferAddChar; + __virBufferContentAndReset; + __virBufferError; __virMacAddrCompare; Index: src/lxc_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/lxc_conf.c,v retrieving revision 1.8 diff -u -p -r1.8 lxc_conf.c --- src/lxc_conf.c 10 Apr 2008 16:54:54 -0000 1.8 +++ src/lxc_conf.c 26 Apr 2008 16:24:40 -0000 @@ -688,99 +688,49 @@ char *lxcGenerateXML(virConnectPtr conn, lxc_vm_t *vm, lxc_vm_def_t *def) { - virBufferPtr buf = 0; + virBuffer buf = VIR_BUFFER_INITIALIZER; unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; lxc_mount_t *mount; - buf = virBufferNew(LXC_MAX_XML_LENGTH); - if (!buf) { - goto no_memory; - } - - if (lxcIsActiveVM(vm)) { - if (virBufferVSprintf(buf, "<domain type='%s' id='%d'>\n", - LXC_DOMAIN_TYPE, vm->def->id) < 0) { - goto no_memory; - } - } else { - if (virBufferVSprintf(buf, "<domain type='%s'>\n", - LXC_DOMAIN_TYPE) < 0) { - goto no_memory; - } - } + if (lxcIsActiveVM(vm)) + virBufferVSprintf(&buf, "<domain type='%s' id='%d'>\n", + LXC_DOMAIN_TYPE, vm->def->id); + else + virBufferVSprintf(&buf, "<domain type='%s'>\n", + LXC_DOMAIN_TYPE); - if (virBufferVSprintf(buf, " <name>%s</name>\n", def->name) < 0) { - goto no_memory; - } + virBufferVSprintf(&buf, " <name>%s</name>\n", def->name); uuid = def->uuid; virUUIDFormat(uuid, uuidstr); - if (virBufferVSprintf(buf, " <uuid>%s</uuid>\n", uuidstr) < 0) { - goto no_memory; - } - - if (virBufferAddLit(buf, " <os>\n") < 0) { - goto no_memory; - } - - if (virBufferVSprintf(buf, " <init>%s</init>\n", def->init) < 0) { - goto no_memory; - } - - if (virBufferAddLit(buf, " </os>\n") < 0) { - goto no_memory; - } - - if (virBufferVSprintf(buf, " <memory>%d</memory>\n", def->maxMemory) < 0) { - goto no_memory; - } - - if (virBufferAddLit(buf, " <devices>\n") < 0) { - goto no_memory; - } + virBufferVSprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferAddLit(&buf, " <os>\n"); + virBufferVSprintf(&buf, " <init>%s</init>\n", def->init); + virBufferAddLit(&buf, " </os>\n"); + virBufferVSprintf(&buf, " <memory>%d</memory>\n", def->maxMemory); + virBufferAddLit(&buf, " <devices>\n"); /* loop adding mounts */ for (mount = def->mounts; mount; mount = mount->next) { - if (virBufferAddLit(buf, " <filesystem type='mount'>\n") < 0) { - goto no_memory; - } - - if (virBufferVSprintf(buf, " <source dir='%s'/>\n", - mount->source) < 0) { - goto no_memory; - } - - if (virBufferVSprintf(buf, " <target dir='%s'/>\n", - mount->target) < 0) { - goto no_memory; - } - - if (virBufferAddLit(buf, " </filesystem>\n") < 0) { - goto no_memory; - } - - } - - if (virBufferVSprintf(buf, " <console tty='%s'/>\n", def->tty) < 0) { - goto no_memory; + virBufferAddLit(&buf, " <filesystem type='mount'>\n"); + virBufferVSprintf(&buf, " <source dir='%s'/>\n", + mount->source); + virBufferVSprintf(&buf, " <target dir='%s'/>\n", + mount->target); + virBufferAddLit(&buf, " </filesystem>\n"); } - if (virBufferAddLit(buf, " </devices>\n") < 0) { - goto no_memory; - } + virBufferVSprintf(&buf, " <console tty='%s'/>\n", def->tty); + virBufferAddLit(&buf, " </devices>\n"); + virBufferAddLit(&buf, "</domain>\n"); - if (virBufferAddLit(buf, "</domain>\n") < 0) { - goto no_memory; + if (virBufferError(&buf)) { + lxcError(conn, NULL, VIR_ERR_NO_MEMORY,_("allocate buffer")); + return NULL; } - return virBufferContentAndFree(buf); - -no_memory: - lxcError(conn, NULL, VIR_ERR_NO_MEMORY, "generateXml"); - virBufferFree(buf); - - return NULL; + return virBufferContentAndReset(&buf); } void lxcFreeVMDef(lxc_vm_def_t *vmdef) Index: src/qemu_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_conf.c,v retrieving revision 1.49 diff -u -p -r1.49 qemu_conf.c --- src/qemu_conf.c 25 Apr 2008 20:46:13 -0000 1.49 +++ src/qemu_conf.c 26 Apr 2008 16:24:42 -0000 @@ -3359,14 +3359,12 @@ static int qemudGenerateXMLChar(virBuffe if (STREQ(type, "console") && dev->srcType == QEMUD_CHR_SRC_TYPE_PTY && dev->srcData.file.path[0] != '\0') { - if (virBufferVSprintf(buf, " <%s type='%s' tty='%s'>\n", - type, types[dev->srcType], - dev->srcData.file.path) < 0) - return -1; + virBufferVSprintf(buf, " <%s type='%s' tty='%s'>\n", + type, types[dev->srcType], + dev->srcData.file.path); } else { - if (virBufferVSprintf(buf, " <%s type='%s'>\n", - type, types[dev->srcType]) < 0) - return -1; + virBufferVSprintf(buf, " <%s type='%s'>\n", + type, types[dev->srcType]); } switch (dev->srcType) { @@ -3382,74 +3380,62 @@ static int qemudGenerateXMLChar(virBuffe case QEMUD_CHR_SRC_TYPE_PIPE: if (dev->srcType != QEMUD_CHR_SRC_TYPE_PTY || dev->srcData.file.path[0]) { - if (virBufferVSprintf(buf, " <source path='%s'/>\n", - dev->srcData.file.path) < 0) - return -1; + virBufferVSprintf(buf, " <source path='%s'/>\n", + dev->srcData.file.path); } break; case QEMUD_CHR_SRC_TYPE_UDP: if (dev->srcData.udp.bindService[0] != '\0' && dev->srcData.udp.bindHost[0] != '\0') { - if (virBufferVSprintf(buf, " <source mode='bind' host='%s' service='%s'/>\n", - dev->srcData.udp.bindHost, - dev->srcData.udp.bindService) < 0) - return -1; + virBufferVSprintf(buf, " <source mode='bind' host='%s' service='%s'/>\n", + dev->srcData.udp.bindHost, + dev->srcData.udp.bindService); } else if (dev->srcData.udp.bindHost[0] !='\0') { - if (virBufferVSprintf(buf, " <source mode='bind' host='%s'/>\n", - dev->srcData.udp.bindHost) < 0) - return -1; + virBufferVSprintf(buf, " <source mode='bind' host='%s'/>\n", + dev->srcData.udp.bindHost); } else if (dev->srcData.udp.bindService[0] != '\0') { - if (virBufferVSprintf(buf, " <source mode='bind' service='%s'/>\n", - dev->srcData.udp.bindService) < 0) - return -1; + virBufferVSprintf(buf, " <source mode='bind' service='%s'/>\n", + dev->srcData.udp.bindService); } if (dev->srcData.udp.connectService[0] != '\0' && dev->srcData.udp.connectHost[0] != '\0') { - if (virBufferVSprintf(buf, " <source mode='connect' host='%s' service='%s'/>\n", - dev->srcData.udp.connectHost, - dev->srcData.udp.connectService) < 0) - return -1; + virBufferVSprintf(buf, " <source mode='connect' host='%s' service='%s'/>\n", + dev->srcData.udp.connectHost, + dev->srcData.udp.connectService); } else if (dev->srcData.udp.connectHost[0] != '\0') { - if (virBufferVSprintf(buf, " <source mode='connect' host='%s'/>\n", - dev->srcData.udp.connectHost) < 0) - return -1; + virBufferVSprintf(buf, " <source mode='connect' host='%s'/>\n", + dev->srcData.udp.connectHost); } else if (dev->srcData.udp.connectService[0] != '\0') { - if (virBufferVSprintf(buf, " <source mode='connect' service='%s'/>\n", - dev->srcData.udp.connectService) < 0) - return -1; + virBufferVSprintf(buf, " <source mode='connect' service='%s'/>\n", + dev->srcData.udp.connectService); } break; case QEMUD_CHR_SRC_TYPE_TCP: - if (virBufferVSprintf(buf, " <source mode='%s' host='%s' service='%s'/>\n", - dev->srcData.tcp.listen ? "bind" : "connect", - dev->srcData.tcp.host, - dev->srcData.tcp.service) < 0) - return -1; - if (virBufferVSprintf(buf, " <protocol type='%s'/>\n", - dev->srcData.tcp.protocol == QEMUD_CHR_SRC_TCP_PROTOCOL_TELNET - ? "telnet" : "raw") < 0) - return -1; + virBufferVSprintf(buf, " <source mode='%s' host='%s' service='%s'/>\n", + dev->srcData.tcp.listen ? "bind" : "connect", + dev->srcData.tcp.host, + dev->srcData.tcp.service); + virBufferVSprintf(buf, " <protocol type='%s'/>\n", + dev->srcData.tcp.protocol == QEMUD_CHR_SRC_TCP_PROTOCOL_TELNET + ? "telnet" : "raw"); break; case QEMUD_CHR_SRC_TYPE_UNIX: - if (virBufferVSprintf(buf, " <source mode='%s' path='%s'/>\n", - dev->srcData.nix.listen ? "bind" : "connect", - dev->srcData.nix.path) < 0) - return -1; + virBufferVSprintf(buf, " <source mode='%s' path='%s'/>\n", + dev->srcData.nix.listen ? "bind" : "connect", + dev->srcData.nix.path); break; } - if (virBufferVSprintf(buf, " <target port='%d'/>\n", - dev->dstPort) < 0) - return -1; + virBufferVSprintf(buf, " <target port='%d'/>\n", + dev->dstPort); - if (virBufferVSprintf(buf, " </%s>\n", - type) < 0) - return -1; + virBufferVSprintf(buf, " </%s>\n", + type); return 0; } @@ -3461,7 +3447,7 @@ char *qemudGenerateXML(virConnectPtr con struct qemud_vm *vm, struct qemud_vm_def *def, int live) { - virBufferPtr buf = 0; + virBuffer buf = VIR_BUFFER_INITIALIZER; unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; const struct qemud_vm_disk_def *disk; @@ -3471,10 +3457,6 @@ char *qemudGenerateXML(virConnectPtr con const char *type = NULL; int n; - buf = virBufferNew (QEMUD_MAX_XML_LEN); - if (!buf) - goto no_memory; - switch (def->virtType) { case QEMUD_VIRT_QEMU: type = "qemu"; @@ -3492,49 +3474,34 @@ char *qemudGenerateXML(virConnectPtr con goto cleanup; } - if (qemudIsActiveVM(vm) && live) { - if (virBufferVSprintf(buf, "<domain type='%s' id='%d'>\n", type, vm->id) < 0) - goto no_memory; - } else { - if (virBufferVSprintf(buf, "<domain type='%s'>\n", type) < 0) - goto no_memory; - } + if (qemudIsActiveVM(vm) && live) + virBufferVSprintf(&buf, "<domain type='%s' id='%d'>\n", type, vm->id); + else + virBufferVSprintf(&buf, "<domain type='%s'>\n", type); - if (virBufferVSprintf(buf, " <name>%s</name>\n", def->name) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <name>%s</name>\n", def->name); uuid = def->uuid; virUUIDFormat(uuid, uuidstr); - if (virBufferVSprintf(buf, " <uuid>%s</uuid>\n", uuidstr) < 0) - goto no_memory; - if (virBufferVSprintf(buf, " <memory>%lu</memory>\n", def->maxmem) < 0) - goto no_memory; - if (virBufferVSprintf(buf, " <currentMemory>%lu</currentMemory>\n", def->memory) < 0) - goto no_memory; - if (virBufferVSprintf(buf, " <vcpu>%d</vcpu>\n", def->vcpus) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); - if (virBufferAddLit(buf, " <os>\n") < 0) - goto no_memory; - - if (def->virtType == QEMUD_VIRT_QEMU) { - if (virBufferVSprintf(buf, " <type arch='%s' machine='%s'>%s</type>\n", - def->os.arch, def->os.machine, def->os.type) < 0) - goto no_memory; - } else { - if (virBufferVSprintf(buf, " <type>%s</type>\n", def->os.type) < 0) - goto no_memory; - } + virBufferVSprintf(&buf, " <memory>%lu</memory>\n", def->maxmem); + virBufferVSprintf(&buf, " <currentMemory>%lu</currentMemory>\n", def->memory); + virBufferVSprintf(&buf, " <vcpu>%d</vcpu>\n", def->vcpus); + virBufferAddLit(&buf, " <os>\n"); + + if (def->virtType == QEMUD_VIRT_QEMU) + virBufferVSprintf(&buf, " <type arch='%s' machine='%s'>%s</type>\n", + def->os.arch, def->os.machine, def->os.type); + else + virBufferVSprintf(&buf, " <type>%s</type>\n", def->os.type); if (def->os.kernel[0]) - if (virBufferVSprintf(buf, " <kernel>%s</kernel>\n", def->os.kernel) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <kernel>%s</kernel>\n", def->os.kernel); if (def->os.initrd[0]) - if (virBufferVSprintf(buf, " <initrd>%s</initrd>\n", def->os.initrd) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <initrd>%s</initrd>\n", def->os.initrd); if (def->os.cmdline[0]) - if (virBufferVSprintf(buf, " <cmdline>%s</cmdline>\n", def->os.cmdline) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <cmdline>%s</cmdline>\n", def->os.cmdline); for (n = 0 ; n < def->os.nBootDevs ; n++) { const char *boottype = "hd"; @@ -3552,41 +3519,29 @@ char *qemudGenerateXML(virConnectPtr con boottype = "network"; break; } - if (virBufferVSprintf(buf, " <boot dev='%s'/>\n", boottype) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <boot dev='%s'/>\n", boottype); } - if (virBufferAddLit(buf, " </os>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " </os>\n"); if (def->features & QEMUD_FEATURE_ACPI) { - if (virBufferAddLit(buf, " <features>\n") < 0) - goto no_memory; - if (virBufferAddLit(buf, " <acpi/>\n") < 0) - goto no_memory; - if (virBufferAddLit(buf, " </features>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " <features>\n"); + virBufferAddLit(&buf, " <acpi/>\n"); + virBufferAddLit(&buf, " </features>\n"); } - virBufferVSprintf(buf, " <clock offset='%s'/>\n", def->localtime ? "localtime" : "utc"); + virBufferVSprintf(&buf, " <clock offset='%s'/>\n", def->localtime ? "localtime" : "utc"); - if (virBufferAddLit(buf, " <on_poweroff>destroy</on_poweroff>\n") < 0) - goto no_memory; - if (def->noReboot) { - if (virBufferAddLit(buf, " <on_reboot>destroy</on_reboot>\n") < 0) - goto no_memory; - } else { - if (virBufferAddLit(buf, " <on_reboot>restart</on_reboot>\n") < 0) - goto no_memory; - } - if (virBufferAddLit(buf, " <on_crash>destroy</on_crash>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " <on_poweroff>destroy</on_poweroff>\n"); + if (def->noReboot) + virBufferAddLit(&buf, " <on_reboot>destroy</on_reboot>\n"); + else + virBufferAddLit(&buf, " <on_reboot>restart</on_reboot>\n"); - if (virBufferAddLit(buf, " <devices>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " <on_crash>destroy</on_crash>\n"); + virBufferAddLit(&buf, " <devices>\n"); - if (virBufferVSprintf(buf, " <emulator>%s</emulator>\n", def->os.binary) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <emulator>%s</emulator>\n", def->os.binary); disk = def->disks; while (disk) { @@ -3603,24 +3558,19 @@ char *qemudGenerateXML(virConnectPtr con "cdrom", "floppy", }; - if (virBufferVSprintf(buf, " <disk type='%s' device='%s'>\n", - types[disk->type], devices[disk->device]) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <disk type='%s' device='%s'>\n", + types[disk->type], devices[disk->device]); if (disk->src[0]) - if (virBufferVSprintf(buf, " <source %s='%s'/>\n", - typeAttrs[disk->type], disk->src) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <source %s='%s'/>\n", + typeAttrs[disk->type], disk->src); - if (virBufferVSprintf(buf, " <target dev='%s'/>\n", disk->dst) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <target dev='%s'/>\n", disk->dst); if (disk->readonly) - if (virBufferAddLit(buf, " <readonly/>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " <readonly/>\n"); - if (virBufferAddLit(buf, " </disk>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " </disk>\n"); disk = disk->next; } @@ -3636,69 +3586,53 @@ char *qemudGenerateXML(virConnectPtr con "network", "bridge", }; - if (virBufferVSprintf(buf, " <interface type='%s'>\n", - types[net->type]) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <interface type='%s'>\n", + types[net->type]); - if (virBufferVSprintf(buf, " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", - net->mac[0], net->mac[1], net->mac[2], - net->mac[3], net->mac[4], net->mac[5]) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", + net->mac[0], net->mac[1], net->mac[2], + net->mac[3], net->mac[4], net->mac[5]); switch (net->type) { case QEMUD_NET_NETWORK: - if (virBufferVSprintf(buf, " <source network='%s'/>\n", net->dst.network.name) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <source network='%s'/>\n", net->dst.network.name); - if (net->dst.network.ifname[0] != '\0') { - if (virBufferVSprintf(buf, " <target dev='%s'/>\n", net->dst.network.ifname) < 0) - goto no_memory; - } + if (net->dst.network.ifname[0] != '\0') + virBufferVSprintf(&buf, " <target dev='%s'/>\n", net->dst.network.ifname); break; case QEMUD_NET_ETHERNET: - if (net->dst.ethernet.ifname[0] != '\0') { - if (virBufferVSprintf(buf, " <target dev='%s'/>\n", net->dst.ethernet.ifname) < 0) - goto no_memory; - } - if (net->dst.ethernet.script[0] != '\0') { - if (virBufferVSprintf(buf, " <script path='%s'/>\n", net->dst.ethernet.script) < 0) - goto no_memory; - } + if (net->dst.ethernet.ifname[0] != '\0') + virBufferVSprintf(&buf, " <target dev='%s'/>\n", net->dst.ethernet.ifname); + if (net->dst.ethernet.script[0] != '\0') + virBufferVSprintf(&buf, " <script path='%s'/>\n", net->dst.ethernet.script); break; case QEMUD_NET_BRIDGE: - if (virBufferVSprintf(buf, " <source bridge='%s'/>\n", net->dst.bridge.brname) < 0) - goto no_memory; - if (net->dst.bridge.ifname[0] != '\0') { - if (virBufferVSprintf(buf, " <target dev='%s'/>\n", net->dst.bridge.ifname) < 0) - goto no_memory; - } + virBufferVSprintf(&buf, " <source bridge='%s'/>\n", net->dst.bridge.brname); + if (net->dst.bridge.ifname[0] != '\0') + virBufferVSprintf(&buf, " <target dev='%s'/>\n", net->dst.bridge.ifname); break; case QEMUD_NET_SERVER: case QEMUD_NET_CLIENT: case QEMUD_NET_MCAST: - if (net->dst.socket.address[0] != '\0') { - if (virBufferVSprintf(buf, " <source address='%s' port='%d'/>\n", - net->dst.socket.address, net->dst.socket.port) < 0) - goto no_memory; - } else { - if (virBufferVSprintf(buf, " <source port='%d'/>\n", - net->dst.socket.port) < 0) - goto no_memory; - } + if (net->dst.socket.address[0] != '\0') + virBufferVSprintf(&buf, " <source address='%s' port='%d'/>\n", + net->dst.socket.address, net->dst.socket.port); + else + virBufferVSprintf(&buf, " <source port='%d'/>\n", + net->dst.socket.port); } - if (virBufferAddLit(buf, " </interface>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " </interface>\n"); net = net->next; } chr = def->serials; while (chr) { - if (qemudGenerateXMLChar(buf, chr, "serial") < 0) + if (qemudGenerateXMLChar(&buf, chr, "serial") < 0) goto no_memory; chr = chr->next; @@ -3706,7 +3640,7 @@ char *qemudGenerateXML(virConnectPtr con chr = def->parallels; while (chr) { - if (qemudGenerateXMLChar(buf, chr, "parallel") < 0) + if (qemudGenerateXMLChar(&buf, chr, "parallel") < 0) goto no_memory; chr = chr->next; @@ -3714,49 +3648,41 @@ char *qemudGenerateXML(virConnectPtr con /* First serial device is the primary console */ if (def->nserials > 0 && - qemudGenerateXMLChar(buf, def->serials, "console") < 0) + qemudGenerateXMLChar(&buf, def->serials, "console") < 0) goto no_memory; input = def->inputs; while (input) { - if (input->bus != QEMU_INPUT_BUS_PS2 && - virBufferVSprintf(buf, " <input type='%s' bus='usb'/>\n", - input->type == QEMU_INPUT_TYPE_MOUSE ? "mouse" : "tablet") < 0) - goto no_memory; + if (input->bus != QEMU_INPUT_BUS_PS2) + virBufferVSprintf(&buf, " <input type='%s' bus='usb'/>\n", + input->type == QEMU_INPUT_TYPE_MOUSE ? "mouse" : "tablet"); input = input->next; } /* If graphics is enable, add implicit mouse */ if (def->graphicsType != QEMUD_GRAPHICS_NONE) - if (virBufferAddLit(buf, " <input type='mouse' bus='ps2'/>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " <input type='mouse' bus='ps2'/>\n"); switch (def->graphicsType) { case QEMUD_GRAPHICS_VNC: - if (virBufferAddLit(buf, " <graphics type='vnc'") < 0) - goto no_memory; + virBufferAddLit(&buf, " <graphics type='vnc'"); - if (def->vncPort && - virBufferVSprintf(buf, " port='%d'", - qemudIsActiveVM(vm) && live ? def->vncActivePort : def->vncPort) < 0) - goto no_memory; + if (def->vncPort) + virBufferVSprintf(&buf, " port='%d'", + qemudIsActiveVM(vm) && live ? def->vncActivePort : def->vncPort); - if (def->vncListen[0] && - virBufferVSprintf(buf, " listen='%s'", - def->vncListen) < 0) - goto no_memory; + if (def->vncListen[0]) + virBufferVSprintf(&buf, " listen='%s'", + def->vncListen); - if (def->keymap && - virBufferVSprintf(buf, " keymap='%s'", - def->keymap) < 0) - goto no_memory; + if (def->keymap) + virBufferVSprintf(&buf, " keymap='%s'", + def->keymap); - if (virBufferAddLit(buf, "/>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, "/>\n"); break; case QEMUD_GRAPHICS_SDL: - if (virBufferAddLit(buf, " <graphics type='sdl'/>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " <graphics type='sdl'/>\n"); break; case QEMUD_GRAPHICS_NONE: @@ -3764,20 +3690,19 @@ char *qemudGenerateXML(virConnectPtr con break; } - if (virBufferAddLit(buf, " </devices>\n") < 0) - goto no_memory; - + virBufferAddLit(&buf, " </devices>\n"); + virBufferAddLit(&buf, "</domain>\n"); - if (virBufferAddLit(buf, "</domain>\n") < 0) + if (virBufferError(&buf)) goto no_memory; - return virBufferContentAndFree (buf); + return virBufferContentAndReset(&buf); no_memory: qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "%s", _("failed to generate XML: out of memory")); cleanup: - if (buf) virBufferFree (buf); + free(virBufferContentAndReset(&buf)); return NULL; } @@ -3786,89 +3711,73 @@ char *qemudGenerateNetworkXML(virConnect struct qemud_driver *driver ATTRIBUTE_UNUSED, struct qemud_network *network, struct qemud_network_def *def) { - virBufferPtr buf = 0; + virBuffer buf = VIR_BUFFER_INITIALIZER; unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; - buf = virBufferNew (QEMUD_MAX_XML_LEN); - if (!buf) - goto no_memory; + virBufferAddLit(&buf, "<network>\n"); - if (virBufferAddLit(buf, "<network>\n") < 0) - goto no_memory; - - if (virBufferVSprintf(buf, " <name>%s</name>\n", def->name) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <name>%s</name>\n", def->name); uuid = def->uuid; virUUIDFormat(uuid, uuidstr); - if (virBufferVSprintf(buf, " <uuid>%s</uuid>\n", uuidstr) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); if (def->forward) { if (def->forwardDev[0]) { - virBufferVSprintf(buf, " <forward dev='%s' mode='%s'/>\n", + virBufferVSprintf(&buf, " <forward dev='%s' mode='%s'/>\n", def->forwardDev, (def->forwardMode == QEMUD_NET_FORWARD_ROUTE ? "route" : "nat")); } else { - virBufferVSprintf(buf, " <forward mode='%s'/>\n", (def->forwardMode == QEMUD_NET_FORWARD_ROUTE ? "route" : "nat")); + virBufferVSprintf(&buf, " <forward mode='%s'/>\n", (def->forwardMode == QEMUD_NET_FORWARD_ROUTE ? "route" : "nat")); } } - virBufferAddLit(buf, " <bridge"); + virBufferAddLit(&buf, " <bridge"); if (qemudIsActiveNetwork(network)) { - if (virBufferVSprintf(buf, " name='%s'", network->bridge) < 0) - goto no_memory; + virBufferVSprintf(&buf, " name='%s'", network->bridge); } else if (def->bridge[0]) { - if (virBufferVSprintf(buf, " name='%s'", def->bridge) < 0) - goto no_memory; + virBufferVSprintf(&buf, " name='%s'", def->bridge); } - if (virBufferVSprintf(buf, " stp='%s' forwardDelay='%d' />\n", - def->disableSTP ? "off" : "on", - def->forwardDelay) < 0) - goto no_memory; + virBufferVSprintf(&buf, " stp='%s' forwardDelay='%d' />\n", + def->disableSTP ? "off" : "on", + def->forwardDelay); if (def->ipAddress[0] || def->netmask[0]) { - if (virBufferAddLit(buf, " <ip") < 0) - goto no_memory; + virBufferAddLit(&buf, " <ip"); - if (def->ipAddress[0] && - virBufferVSprintf(buf, " address='%s'", def->ipAddress) < 0) - goto no_memory; + if (def->ipAddress[0]) + virBufferVSprintf(&buf, " address='%s'", def->ipAddress); - if (def->netmask[0] && - virBufferVSprintf(buf, " netmask='%s'", def->netmask) < 0) - goto no_memory; + if (def->netmask[0]) + virBufferVSprintf(&buf, " netmask='%s'", def->netmask); - if (virBufferAddLit(buf, ">\n") < 0) - goto no_memory; + virBufferAddLit(&buf, ">\n"); if (def->ranges) { struct qemud_dhcp_range_def *range = def->ranges; - if (virBufferAddLit(buf, " <dhcp>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " <dhcp>\n"); while (range) { - if (virBufferVSprintf(buf, " <range start='%s' end='%s' />\n", - range->start, range->end) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <range start='%s' end='%s' />\n", + range->start, range->end); range = range->next; } - if (virBufferAddLit(buf, " </dhcp>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " </dhcp>\n"); } - if (virBufferAddLit(buf, " </ip>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " </ip>\n"); } - if (virBufferAddLit(buf, "</network>\n") < 0) + virBufferAddLit(&buf, "</network>\n"); + + if (virBufferError(&buf)) goto no_memory; - return virBufferContentAndFree (buf); + return virBufferContentAndReset(&buf); no_memory: qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "%s", _("failed to generate XML: out of memory")); - if (buf) virBufferFree (buf); + free(virBufferContentAndReset(&buf)); return NULL; } Index: src/qparams.c =================================================================== RCS file: /data/cvs/libvirt/src/qparams.c,v retrieving revision 1.3 diff -u -p -r1.3 qparams.c --- src/qparams.c 10 Apr 2008 16:53:29 -0000 1.3 +++ src/qparams.c 26 Apr 2008 16:24:43 -0000 @@ -136,20 +136,23 @@ append_qparam (struct qparam_set *ps, char * qparam_get_query (const struct qparam_set *ps) { - virBufferPtr buf; + virBuffer buf = VIR_BUFFER_INITIALIZER; int i, amp = 0; - buf = virBufferNew (100); for (i = 0; i < ps->n; ++i) { if (!ps->p[i].ignore) { - if (amp) virBufferAddChar (buf, '&'); - virBufferStrcat (buf, ps->p[i].name, "=", NULL); - virBufferURIEncodeString (buf, ps->p[i].value); + if (amp) virBufferAddChar (&buf, '&'); + virBufferStrcat (&buf, ps->p[i].name, "=", NULL); + virBufferURIEncodeString (&buf, ps->p[i].value); amp = 1; } } - return virBufferContentAndFree (buf); + if (virBufferError(&buf)) { + return NULL; + } + + return virBufferContentAndReset(&buf); } void Index: src/storage_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/storage_conf.c,v retrieving revision 1.5 diff -u -p -r1.5 storage_conf.c --- src/storage_conf.c 10 Apr 2008 16:53:29 -0000 1.5 +++ src/storage_conf.c 26 Apr 2008 16:24:43 -0000 @@ -414,7 +414,7 @@ char * virStoragePoolDefFormat(virConnectPtr conn, virStoragePoolDefPtr def) { virStorageBackendPoolOptionsPtr options; - virBufferPtr buf; + virBuffer buf = VIR_BUFFER_INITIALIZER; const char *type; char uuid[VIR_UUID_STRING_BUFLEN]; int i; @@ -423,126 +423,96 @@ virStoragePoolDefFormat(virConnectPtr co if (options == NULL) return NULL; - if ((buf = virBufferNew(8192)) == NULL) - goto no_memory; - type = virStorageBackendToString(def->type); if (!type) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected pool type")); goto cleanup; } - if (virBufferVSprintf(buf, "<pool type='%s'>\n", type) < 0) - goto no_memory; - - if (virBufferVSprintf(buf," <name>%s</name>\n", def->name) < 0) - goto no_memory; + virBufferVSprintf(&buf, "<pool type='%s'>\n", type); + virBufferVSprintf(&buf," <name>%s</name>\n", def->name); virUUIDFormat(def->uuid, uuid); - if (virBufferVSprintf(buf," <uuid>%s</uuid>\n", uuid) < 0) - goto no_memory; - - if (virBufferVSprintf(buf," <capacity>%llu</capacity>\n", - def->capacity) < 0) - goto no_memory; - if (virBufferVSprintf(buf," <allocation>%llu</allocation>\n", - def->allocation) < 0) - goto no_memory; - if (virBufferVSprintf(buf," <available>%llu</available>\n", - def->available) < 0) - goto no_memory; + virBufferVSprintf(&buf," <uuid>%s</uuid>\n", uuid); + virBufferVSprintf(&buf," <capacity>%llu</capacity>\n", + def->capacity); + virBufferVSprintf(&buf," <allocation>%llu</allocation>\n", + def->allocation); + virBufferVSprintf(&buf," <available>%llu</available>\n", + def->available); - if (virBufferAddLit(buf," <source>\n") < 0) - goto no_memory; + virBufferAddLit(&buf," <source>\n"); if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_HOST) && - def->source.host.name && - virBufferVSprintf(buf," <host name='%s'/>\n", def->source.host.name) < 0) - goto no_memory; + def->source.host.name) + virBufferVSprintf(&buf," <host name='%s'/>\n", def->source.host.name); + if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE) && def->source.ndevice) { for (i = 0 ; i < def->source.ndevice ; i++) { - if (virBufferVSprintf(buf," <device path='%s'>\n", def->source.devices[i].path) < 0) - goto no_memory; + virBufferVSprintf(&buf," <device path='%s'>\n", def->source.devices[i].path); if (def->source.devices[i].nfreeExtent) { int j; for (j = 0 ; j < def->source.devices[i].nfreeExtent ; j++) { - if (virBufferVSprintf(buf, " <freeExtent start='%llu' end='%llu'/>\n", - def->source.devices[i].freeExtents[j].start, - def->source.devices[i].freeExtents[j].end) < 0) - goto no_memory; + virBufferVSprintf(&buf, " <freeExtent start='%llu' end='%llu'/>\n", + def->source.devices[i].freeExtents[j].start, + def->source.devices[i].freeExtents[j].end); } } - if (virBufferAddLit(buf," </device>\n") < 0) - goto no_memory; + virBufferAddLit(&buf," </device>\n"); } } if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_DIR) && - def->source.dir && - virBufferVSprintf(buf," <dir path='%s'/>\n", def->source.dir) < 0) - goto no_memory; + def->source.dir) + virBufferVSprintf(&buf," <dir path='%s'/>\n", def->source.dir); if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_ADAPTER) && - def->source.adapter && - virBufferVSprintf(buf," <adapter name='%s'/>\n", def->source.adapter) < 0) - goto no_memory; + def->source.adapter) + virBufferVSprintf(&buf," <adapter name='%s'/>\n", def->source.adapter); if (options->formatToString) { const char *format = (options->formatToString)(conn, def->source.format); if (!format) goto cleanup; - if (virBufferVSprintf(buf," <format type='%s'/>\n", format) < 0) - goto no_memory; + virBufferVSprintf(&buf," <format type='%s'/>\n", format); } - if (def->source.authType == VIR_STORAGE_POOL_AUTH_CHAP && - virBufferVSprintf(buf," <auth type='chap' login='%s' passwd='%s'>\n", + if (def->source.authType == VIR_STORAGE_POOL_AUTH_CHAP) + virBufferVSprintf(&buf," <auth type='chap' login='%s' passwd='%s'>\n", def->source.auth.chap.login, - def->source.auth.chap.passwd) < 0) - goto no_memory; - if (virBufferAddLit(buf," </source>\n") < 0) - goto no_memory; + def->source.auth.chap.passwd); + virBufferAddLit(&buf," </source>\n"); + virBufferAddLit(&buf," <target>\n"); + if (def->target.path) + virBufferVSprintf(&buf," <path>%s</path>\n", def->target.path); - if (virBufferAddLit(buf," <target>\n") < 0) - goto no_memory; + virBufferAddLit(&buf," <permissions>\n"); + virBufferVSprintf(&buf," <mode>0%o</mode>\n", + def->target.perms.mode); + virBufferVSprintf(&buf," <owner>%d</owner>\n", + def->target.perms.uid); + virBufferVSprintf(&buf," <group>%d</group>\n", + def->target.perms.gid); - if (def->target.path && - virBufferVSprintf(buf," <path>%s</path>\n", def->target.path) < 0) - goto no_memory; + if (def->target.perms.label) + virBufferVSprintf(&buf," <label>%s</label>\n", + def->target.perms.label); - if (virBufferAddLit(buf," <permissions>\n") < 0) - goto no_memory; - if (virBufferVSprintf(buf," <mode>0%o</mode>\n", - def->target.perms.mode) < 0) - goto no_memory; - if (virBufferVSprintf(buf," <owner>%d</owner>\n", - def->target.perms.uid) < 0) - goto no_memory; - if (virBufferVSprintf(buf," <group>%d</group>\n", - def->target.perms.gid) < 0) - goto no_memory; + virBufferAddLit(&buf," </permissions>\n"); + virBufferAddLit(&buf," </target>\n"); + virBufferAddLit(&buf,"</pool>\n"); - if (def->target.perms.label) { - if (virBufferVSprintf(buf," <label>%s</label>\n", - def->target.perms.label) < 0) - goto no_memory; - } - if (virBufferAddLit(buf," </permissions>\n") < 0) - goto no_memory; - if (virBufferAddLit(buf," </target>\n") < 0) + if (virBufferError(&buf)) goto no_memory; - if (virBufferAddLit(buf,"</pool>\n") < 0) - goto no_memory; - - return virBufferContentAndFree(buf); + return virBufferContentAndReset(&buf); no_memory: virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("xml")); cleanup: - virBufferFree(buf); + free(virBufferContentAndReset(&buf)); return NULL; } @@ -804,26 +774,17 @@ virStorageVolDefFormat(virConnectPtr con virStoragePoolDefPtr pool, virStorageVolDefPtr def) { virStorageBackendVolOptionsPtr options; - virBufferPtr buf = virBufferNew(8192); + virBuffer buf = VIR_BUFFER_INITIALIZER; options = virStorageBackendVolOptionsForType(pool->type); if (options == NULL) return NULL; - if (!buf) - goto no_memory; - - if (virBufferAddLit(buf, "<volume>\n") < 0) - goto no_memory; - - if (virBufferVSprintf(buf," <name>%s</name>\n", def->name) < 0) - goto no_memory; - - if (virBufferVSprintf(buf," <key>%s</key>\n", def->key) < 0) - goto no_memory; + virBufferAddLit(&buf, "<volume>\n"); + virBufferVSprintf(&buf," <name>%s</name>\n", def->name); + virBufferVSprintf(&buf," <key>%s</key>\n", def->key); + virBufferAddLit(&buf, " <source>\n"); - if (virBufferAddLit(buf, " <source>\n") < 0) - goto no_memory; if (def->source.nextent) { int i; const char *thispath = NULL; @@ -831,81 +792,67 @@ virStorageVolDefFormat(virConnectPtr con if (thispath == NULL || STRNEQ(thispath, def->source.extents[i].path)) { if (thispath != NULL) - if (virBufferAddLit(buf, " </device>\n") < 0) - goto no_memory; - if (virBufferVSprintf(buf, " <device path='%s'>\n", - def->source.extents[i].path) < 0) - goto no_memory; + virBufferAddLit(&buf, " </device>\n"); + + virBufferVSprintf(&buf, " <device path='%s'>\n", + def->source.extents[i].path); } - if (virBufferVSprintf(buf, - " <extent start='%llu' end='%llu'/>\n", - def->source.extents[i].start, - def->source.extents[i].end) < 0) - goto no_memory; + virBufferVSprintf(&buf, + " <extent start='%llu' end='%llu'/>\n", + def->source.extents[i].start, + def->source.extents[i].end); thispath = def->source.extents[i].path; } if (thispath != NULL) - if (virBufferAddLit(buf, " </device>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " </device>\n"); } - if (virBufferAddLit(buf, " </source>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " </source>\n"); - if (virBufferVSprintf(buf," <capacity>%llu</capacity>\n", - def->capacity) < 0) - goto no_memory; - if (virBufferVSprintf(buf," <allocation>%llu</allocation>\n", - def->allocation) < 0) - goto no_memory; + virBufferVSprintf(&buf," <capacity>%llu</capacity>\n", + def->capacity); + virBufferVSprintf(&buf," <allocation>%llu</allocation>\n", + def->allocation); - if (virBufferAddLit(buf, " <target>\n") < 0) - goto no_memory; + virBufferAddLit(&buf, " <target>\n"); - if (def->target.path && - virBufferVSprintf(buf," <path>%s</path>\n", def->target.path) < 0) - goto no_memory; + if (def->target.path) + virBufferVSprintf(&buf," <path>%s</path>\n", def->target.path); if (options->formatToString) { const char *format = (options->formatToString)(conn, def->target.format); if (!format) goto cleanup; - if (virBufferVSprintf(buf," <format type='%s'/>\n", format) < 0) - goto no_memory; + virBufferVSprintf(&buf," <format type='%s'/>\n", format); } - if (virBufferAddLit(buf," <permissions>\n") < 0) - goto no_memory; - if (virBufferVSprintf(buf," <mode>0%o</mode>\n", - def->target.perms.mode) < 0) - goto no_memory; - if (virBufferVSprintf(buf," <owner>%d</owner>\n", - def->target.perms.uid) < 0) - goto no_memory; - if (virBufferVSprintf(buf," <group>%d</group>\n", - def->target.perms.gid) < 0) - goto no_memory; + virBufferAddLit(&buf," <permissions>\n"); + virBufferVSprintf(&buf," <mode>0%o</mode>\n", + def->target.perms.mode); + virBufferVSprintf(&buf," <owner>%d</owner>\n", + def->target.perms.uid); + virBufferVSprintf(&buf," <group>%d</group>\n", + def->target.perms.gid); - if (def->target.perms.label && - virBufferVSprintf(buf," <label>%s</label>\n", - def->target.perms.label) < 0) - goto no_memory; - if (virBufferAddLit(buf," </permissions>\n") < 0) - goto no_memory; - if (virBufferAddLit(buf, " </target>\n") < 0) - goto no_memory; + if (def->target.perms.label) + virBufferVSprintf(&buf," <label>%s</label>\n", + def->target.perms.label); + + virBufferAddLit(&buf," </permissions>\n"); + virBufferAddLit(&buf, " </target>\n"); + virBufferAddLit(&buf,"</volume>\n"); - if (virBufferAddLit(buf,"</volume>\n") < 0) + if (virBufferError(&buf)) goto no_memory; - return virBufferContentAndFree(buf); + return virBufferContentAndReset(&buf); no_memory: virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("xml")); cleanup: - virBufferFree(buf); + free(virBufferContentAndReset(&buf)); return NULL; } Index: src/test.c =================================================================== RCS file: /data/cvs/libvirt/src/test.c,v retrieving revision 1.72 diff -u -p -r1.72 test.c --- src/test.c 10 Apr 2008 16:53:29 -0000 1.72 +++ src/test.c 26 Apr 2008 16:24:45 -0000 @@ -1523,34 +1523,30 @@ static int testSetVcpus(virDomainPtr dom static char *testDomainDumpXML(virDomainPtr domain, int flags ATTRIBUTE_UNUSED) { - virBufferPtr buf; - char *xml; + virBuffer buf = VIR_BUFFER_INITIALIZER; unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; GET_DOMAIN(domain, NULL); - if (!(buf = virBufferNew(4000))) { - testError(domain->conn, domain, NULL, VIR_ERR_NO_MEMORY, __FUNCTION__); - return (NULL); - } - - virBufferVSprintf(buf, "<domain type='test' id='%d'>\n", domain->id); - virBufferVSprintf(buf, " <name>%s</name>\n", domain->name); + virBufferVSprintf(&buf, "<domain type='test' id='%d'>\n", domain->id); + virBufferVSprintf(&buf, " <name>%s</name>\n", domain->name); uuid = domain->uuid; virUUIDFormat(uuid, uuidstr); - virBufferVSprintf(buf, " <uuid>%s</uuid>\n", uuidstr); - virBufferVSprintf(buf, " <memory>%lu</memory>\n", privdom->info.maxMem); - virBufferVSprintf(buf, " <vcpu>%d</vcpu>\n", privdom->info.nrVirtCpu); - virBufferVSprintf(buf, " <on_reboot>%s</on_reboot>\n", testRestartFlagToString(privdom->onReboot)); - virBufferVSprintf(buf, " <on_poweroff>%s</on_poweroff>\n", testRestartFlagToString(privdom->onPoweroff)); - virBufferVSprintf(buf, " <on_crash>%s</on_crash>\n", testRestartFlagToString(privdom->onCrash)); + virBufferVSprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferVSprintf(&buf, " <memory>%lu</memory>\n", privdom->info.maxMem); + virBufferVSprintf(&buf, " <vcpu>%d</vcpu>\n", privdom->info.nrVirtCpu); + virBufferVSprintf(&buf, " <on_reboot>%s</on_reboot>\n", testRestartFlagToString(privdom->onReboot)); + virBufferVSprintf(&buf, " <on_poweroff>%s</on_poweroff>\n", testRestartFlagToString(privdom->onPoweroff)); + virBufferVSprintf(&buf, " <on_crash>%s</on_crash>\n", testRestartFlagToString(privdom->onCrash)); - virBufferAddLit(buf, "</domain>\n"); + virBufferAddLit(&buf, "</domain>\n"); - xml = buf->content; - free(buf); + if (virBufferError(&buf)) { + testError(domain->conn, domain, NULL, VIR_ERR_NO_MEMORY, __FUNCTION__); + return NULL; + } - return (xml); + return virBufferContentAndReset(&buf); } static int testNumOfDefinedDomains(virConnectPtr conn) { @@ -1928,44 +1924,40 @@ static int testNetworkDestroy(virNetwork } static char *testNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSED) { - virBufferPtr buf; - char *xml; + virBuffer buf = VIR_BUFFER_INITIALIZER; unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; GET_NETWORK(network, NULL); - if (!(buf = virBufferNew(4000))) { - testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY, __FUNCTION__); - return (NULL); - } - - virBufferAddLit(buf, "<network>\n"); - virBufferVSprintf(buf, " <name>%s</name>\n", network->name); + virBufferAddLit(&buf, "<network>\n"); + virBufferVSprintf(&buf, " <name>%s</name>\n", network->name); uuid = network->uuid; virUUIDFormat(uuid, uuidstr); - virBufferVSprintf(buf, " <uuid>%s</uuid>\n", uuidstr); - virBufferVSprintf(buf, " <bridge name='%s'/>\n", privnet->bridge); + virBufferVSprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferVSprintf(&buf, " <bridge name='%s'/>\n", privnet->bridge); if (privnet->forward) { if (privnet->forwardDev[0]) - virBufferVSprintf(buf, " <forward dev='%s'/>\n", privnet->forwardDev); + virBufferVSprintf(&buf, " <forward dev='%s'/>\n", privnet->forwardDev); else - virBufferAddLit(buf, " <forward/>\n"); + virBufferAddLit(&buf, " <forward/>\n"); } - virBufferVSprintf(buf, " <ip address='%s' netmask='%s'>\n", + virBufferVSprintf(&buf, " <ip address='%s' netmask='%s'>\n", privnet->ipAddress, privnet->ipNetmask); - virBufferAddLit(buf, " <dhcp>\n"); - virBufferVSprintf(buf, " <range start='%s' end='%s'/>\n", + virBufferAddLit(&buf, " <dhcp>\n"); + virBufferVSprintf(&buf, " <range start='%s' end='%s'/>\n", privnet->dhcpStart, privnet->dhcpEnd); - virBufferAddLit(buf, " </dhcp>\n"); - virBufferAddLit(buf, " </ip>\n"); + virBufferAddLit(&buf, " </dhcp>\n"); + virBufferAddLit(&buf, " </ip>\n"); - virBufferAddLit(buf, "</network>\n"); + virBufferAddLit(&buf, "</network>\n"); - xml = buf->content; - free(buf); + if (virBufferError(&buf)) { + testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY, __FUNCTION__); + return NULL; + } - return (xml); + return virBufferContentAndReset(&buf); } static char *testNetworkGetBridgeName(virNetworkPtr network) { Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.146 diff -u -p -r1.146 virsh.c --- src/virsh.c 25 Apr 2008 14:53:05 -0000 1.146 +++ src/virsh.c 26 Apr 2008 16:24:49 -0000 @@ -2885,10 +2885,9 @@ cmdPoolCreateAs(vshControl * ctl, vshCmd { virStoragePoolPtr pool; int found; + char *xml; char *name, *type, *srcHost, *srcPath, *srcDev, *target; - virBuffer buf; - - memset(&buf, 0, sizeof(buf)); + virBuffer buf = VIR_BUFFER_INITIALIZER; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -2905,39 +2904,36 @@ cmdPoolCreateAs(vshControl * ctl, vshCmd srcDev = vshCommandOptString(cmd, "source-dev", &found); target = vshCommandOptString(cmd, "target", &found); - if (virBufferVSprintf(&buf, "<pool type='%s'>\n", type) < 0) - goto cleanup; - if (virBufferVSprintf(&buf, " <name>%s</name>\n", name) < 0) - goto cleanup; + virBufferVSprintf(&buf, "<pool type='%s'>\n", type); + virBufferVSprintf(&buf, " <name>%s</name>\n", name); if (srcHost || srcPath || srcDev) { - if (virBufferAddLit(&buf, " <source>\n") < 0) - goto cleanup; - if (srcHost && - virBufferVSprintf(&buf, " <host name='%s'>\n", srcHost) < 0) - goto cleanup; - if (srcPath && - virBufferVSprintf(&buf, " <dir path='%s'/>\n", srcPath) < 0) - goto cleanup; - if (srcDev && - virBufferVSprintf(&buf, " <device path='%s'/>\n", srcDev) < 0) - goto cleanup; + virBufferAddLit(&buf, " <source>\n"); + if (srcHost) + virBufferVSprintf(&buf, " <host name='%s'>\n", srcHost); - if (virBufferAddLit(&buf, " </source>\n") < 0) - goto cleanup; + if (srcPath) + virBufferVSprintf(&buf, " <dir path='%s'/>\n", srcPath); + + if (srcDev) + virBufferVSprintf(&buf, " <device path='%s'/>\n", srcDev); + + virBufferAddLit(&buf, " </source>\n"); } if (target) { - if (virBufferAddLit(&buf, " <target>\n") < 0) - goto cleanup; - if (virBufferVSprintf(&buf, " <path>%s</path>\n", target) < 0) - goto cleanup; - if (virBufferAddLit(&buf, " </target>\n") < 0) - goto cleanup; + virBufferAddLit(&buf, " <target>\n"); + virBufferVSprintf(&buf, " <path>%s</path>\n", target); + virBufferAddLit(&buf, " </target>\n"); } - if (virBufferAddLit(&buf, "</pool>\n") < 0) - goto cleanup; + virBufferAddLit(&buf, "</pool>\n"); + + if (virBufferError(&buf)) { + vshPrint(ctl, "%s", _("Failed to allocate XML buffer")); + return FALSE; + } + xml = virBufferContentAndReset(&buf); - pool = virStoragePoolCreateXML(ctl->conn, buf.content, 0); - free (buf.content); + pool = virStoragePoolCreateXML(ctl->conn, xml, 0); + free (xml); if (pool != NULL) { vshPrint(ctl, _("Pool %s created\n"), name); @@ -2949,7 +2945,7 @@ cmdPoolCreateAs(vshControl * ctl, vshCmd } cleanup: - free(buf.content); + free(virBufferContentAndReset(&buf)); return FALSE; } @@ -3028,10 +3024,9 @@ cmdPoolDefineAs(vshControl * ctl, vshCmd { virStoragePoolPtr pool; int found; + char *xml; char *name, *type, *srcHost, *srcPath, *srcDev, *target; - virBuffer buf; - - memset(&buf, 0, sizeof(buf)); + virBuffer buf = VIR_BUFFER_INITIALIZER; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -3048,39 +3043,35 @@ cmdPoolDefineAs(vshControl * ctl, vshCmd srcDev = vshCommandOptString(cmd, "source-dev", &found); target = vshCommandOptString(cmd, "target", &found); - if (virBufferVSprintf(&buf, "<pool type='%s'>\n", type) < 0) - goto cleanup; - if (virBufferVSprintf(&buf, " <name>%s</name>\n", name) < 0) - goto cleanup; + virBufferVSprintf(&buf, "<pool type='%s'>\n", type); + virBufferVSprintf(&buf, " <name>%s</name>\n", name); if (srcHost || srcPath || srcDev) { - if (virBufferAddLit(&buf, " <source>\n") < 0) - goto cleanup; - if (srcHost && - virBufferVSprintf(&buf, " <host>%s</host>\n", srcHost) < 0) - goto cleanup; - if (srcPath && - virBufferVSprintf(&buf, " <path>%s</path>\n", srcPath) < 0) - goto cleanup; - if (srcDev && - virBufferVSprintf(&buf, " <device>%s</device>\n", srcDev) < 0) - goto cleanup; + virBufferAddLit(&buf, " <source>\n"); + if (srcHost) + virBufferVSprintf(&buf, " <host>%s</host>\n", srcHost); + if (srcPath) + virBufferVSprintf(&buf, " <path>%s</path>\n", srcPath); + if (srcDev) + virBufferVSprintf(&buf, " <device>%s</device>\n", srcDev); - if (virBufferAddLit(&buf, " </source>\n") < 0) - goto cleanup; + virBufferAddLit(&buf, " </source>\n"); } if (target) { - if (virBufferAddLit(&buf, " <target>\n") < 0) - goto cleanup; - if (virBufferVSprintf(&buf, " <path>%s</path>\n", target) < 0) - goto cleanup; - if (virBufferAddLit(&buf, " </target>\n") < 0) - goto cleanup; + virBufferAddLit(&buf, " <target>\n"); + virBufferVSprintf(&buf, " <path>%s</path>\n", target); + virBufferAddLit(&buf, " </target>\n"); } - if (virBufferAddLit(&buf, "</pool>\n") < 0) - goto cleanup; + virBufferAddLit(&buf, "</pool>\n"); - pool = virStoragePoolDefineXML(ctl->conn, buf.content, 0); - free (buf.content); + + if (virBufferError(&buf)) { + vshPrint(ctl, "%s", _("Failed to allocate XML buffer")); + return FALSE; + } + xml = virBufferContentAndReset(&buf); + + pool = virStoragePoolDefineXML(ctl->conn, xml, 0); + free (xml); if (pool != NULL) { vshPrint(ctl, _("Pool %s defined\n"), name); @@ -3092,7 +3083,7 @@ cmdPoolDefineAs(vshControl * ctl, vshCmd } cleanup: - free(buf.content); + free(virBufferContentAndReset(&buf)); return FALSE; } @@ -3641,11 +3632,10 @@ cmdVolCreateAs(vshControl * ctl, vshCmd virStoragePoolPtr pool; virStorageVolPtr vol; int found; + char *xml; char *name, *capacityStr, *allocationStr, *format; unsigned long long capacity, allocation = 0; - virBuffer buf; - - memset(&buf, 0, sizeof(buf)); + virBuffer buf = VIR_BUFFER_INITIALIZER; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -3671,30 +3661,28 @@ cmdVolCreateAs(vshControl * ctl, vshCmd format = vshCommandOptString(cmd, "format", &found); - if (virBufferAddLit(&buf, "<volume>\n") < 0) - goto cleanup; - if (virBufferVSprintf(&buf, " <name>%s</name>\n", name) < 0) - goto cleanup; - if (virBufferVSprintf(&buf, " <capacity>%llu</capacity>\n", capacity) < 0) - goto cleanup; - if (allocationStr && - virBufferVSprintf(&buf, " <allocation>%llu</allocation>\n", allocation) < 0) - goto cleanup; + virBufferAddLit(&buf, "<volume>\n"); + virBufferVSprintf(&buf, " <name>%s</name>\n", name); + virBufferVSprintf(&buf, " <capacity>%llu</capacity>\n", capacity); + if (allocationStr) + virBufferVSprintf(&buf, " <allocation>%llu</allocation>\n", allocation); if (format) { - if (virBufferAddLit(&buf, " <target>\n") < 0) - goto cleanup; + virBufferAddLit(&buf, " <target>\n"); if (format) - if (virBufferVSprintf(&buf, " <format type='%s'/>\n",format) < 0) - goto cleanup; - if (virBufferAddLit(&buf, " </target>\n") < 0) - goto cleanup; + virBufferVSprintf(&buf, " <format type='%s'/>\n",format); + virBufferAddLit(&buf, " </target>\n"); } - if (virBufferAddLit(&buf, "</volume>\n") < 0) - goto cleanup; + virBufferAddLit(&buf, "</volume>\n"); + - vol = virStorageVolCreateXML(pool, buf.content, 0); - free (buf.content); + if (virBufferError(&buf)) { + vshPrint(ctl, "%s", _("Failed to allocate XML buffer")); + return FALSE; + } + xml = virBufferContentAndReset(&buf); + vol = virStorageVolCreateXML(pool, xml, 0); + free (xml); virStoragePoolFree(pool); if (vol != NULL) { @@ -3707,7 +3695,7 @@ cmdVolCreateAs(vshControl * ctl, vshCmd } cleanup: - free(buf.content); + free(virBufferContentAndReset(&buf)); virStoragePoolFree(pool); return FALSE; } Index: src/xend_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xend_internal.c,v retrieving revision 1.181 diff -u -p -r1.181 xend_internal.c --- src/xend_internal.c 26 Apr 2008 14:22:02 -0000 1.181 +++ src/xend_internal.c 26 Apr 2008 16:24:51 -0000 @@ -608,16 +608,9 @@ xend_op_ext2(virConnectPtr xend, const c size_t n_error, const char *key, va_list ap) { const char *k = key, *v; - virBuffer buf; + virBuffer buf = VIR_BUFFER_INITIALIZER; int ret; - - buf.content = malloc(1000); - if (buf.content == NULL) { - virXendError(xend, VIR_ERR_NO_MEMORY, _("allocate new buffer")); - return -1; - } - buf.size = 1000; - buf.use = 0; + char *content; while (k) { v = va_arg(ap, const char *); @@ -631,8 +624,14 @@ xend_op_ext2(virConnectPtr xend, const c virBufferVSprintf(&buf, "%s", "&"); } - ret = http2unix(xend, xend_post(xend, path, buf.content, error, n_error)); - free(buf.content); + if (virBufferError(&buf)) { + virXendError(NULL, VIR_ERR_NO_MEMORY, _("allocate buffer")); + return -1; + } + + content = virBufferContentAndReset(&buf); + ret = http2unix(xend, xend_post(xend, path, content, error, n_error)); + free(content); return ret; } @@ -1437,13 +1436,11 @@ xend_parse_sexp_desc_char(virConnectPtr if (STREQ(devtype, "console") && STREQ(type, "pty") && tty != NULL) { - if (virBufferVSprintf(buf, " <%s type='%s' tty='%s'>\n", - devtype, type, tty) < 0) - goto no_memory; + virBufferVSprintf(buf, " <%s type='%s' tty='%s'>\n", + devtype, type, tty); } else { - if (virBufferVSprintf(buf, " <%s type='%s'>\n", - devtype, type) < 0) - goto no_memory; + virBufferVSprintf(buf, " <%s type='%s'>\n", + devtype, type); } if (STREQ(type, "null") || @@ -1451,15 +1448,13 @@ xend_parse_sexp_desc_char(virConnectPtr STREQ(type, "stdio")) { /* no source needed */ } else if (STREQ(type, "pty")) { - if (tty && + if (tty) virBufferVSprintf(buf, " <source path='%s'/>\n", - tty) < 0) - goto no_memory; + tty); } else if (STREQ(type, "file") || STREQ(type, "pipe")) { - if (virBufferVSprintf(buf, " <source path='%s'/>\n", - value) < 0) - goto no_memory; + virBufferVSprintf(buf, " <source path='%s'/>\n", + value); } else if (STREQ(type, "tcp")) { const char *offset = strchr(value, ':'); const char *offset2; @@ -1490,20 +1485,17 @@ xend_parse_sexp_desc_char(virConnectPtr protocol = telnet ? "telnet":"raw"; if (bindHost) { - if (virBufferVSprintf(buf, - " <source mode='%s' host='%s' service='%s'/>\n", - mode, bindHost, bindPort) < 0) - goto no_memory; + virBufferVSprintf(buf, + " <source mode='%s' host='%s' service='%s'/>\n", + mode, bindHost, bindPort); } else { - if (virBufferVSprintf(buf, - " <source mode='%s' service='%s'/>\n", - mode, bindPort) < 0) - goto no_memory; - } - if (virBufferVSprintf(buf, - " <protocol type='%s'/>\n", - protocol) < 0) - goto no_memory; + virBufferVSprintf(buf, + " <source mode='%s' service='%s'/>\n", + mode, bindPort); + } + virBufferVSprintf(buf, + " <protocol type='%s'/>\n", + protocol); } else if (STREQ(type, "udp")) { const char *offset = strchr(value, ':'); const char *offset2, *offset3; @@ -1543,28 +1535,24 @@ xend_parse_sexp_desc_char(virConnectPtr if (connectPort) { if (connectHost) { - if (virBufferVSprintf(buf, - " <source mode='connect' host='%s' service='%s'/>\n", - connectHost, connectPort) < 0) - goto no_memory; + virBufferVSprintf(buf, + " <source mode='connect' host='%s' service='%s'/>\n", + connectHost, connectPort); } else { - if (virBufferVSprintf(buf, - " <source mode='connect' service='%s'/>\n", - connectPort) < 0) - goto no_memory; + virBufferVSprintf(buf, + " <source mode='connect' service='%s'/>\n", + connectPort); } } if (bindPort) { if (bindHost) { - if (virBufferVSprintf(buf, - " <source mode='bind' host='%s' service='%s'/>\n", - bindHost, bindPort) < 0) - goto no_memory; + virBufferVSprintf(buf, + " <source mode='bind' host='%s' service='%s'/>\n", + bindHost, bindPort); } else { - if (virBufferVSprintf(buf, - " <source mode='bind' service='%s'/>\n", - bindPort) < 0) - goto no_memory; + virBufferVSprintf(buf, + " <source mode='bind' service='%s'/>\n", + bindPort); } } @@ -1582,18 +1570,15 @@ xend_parse_sexp_desc_char(virConnectPtr strstr(offset, ",listen") != NULL) dolisten = 1; - if (virBufferVSprintf(buf, " <source mode='%s' path='%s'/>\n", - dolisten ? "bind" : "connect", path) < 0) - goto no_memory; + virBufferVSprintf(buf, " <source mode='%s' path='%s'/>\n", + dolisten ? "bind" : "connect", path); } - if (virBufferVSprintf(buf, " <target port='%d'/>\n", - portNum) < 0) - goto no_memory; - - if (virBufferVSprintf(buf, " </%s>\n", - devtype) < 0) - goto no_memory; + virBufferVSprintf(buf, " <target port='%d'/>\n", + portNum); + + virBufferVSprintf(buf, " </%s>\n", + devtype); ret = 0; @@ -1635,7 +1620,7 @@ xend_parse_sexp_desc(virConnectPtr conn, struct sexpr *cur, *node; const char *tmp; char *tty; - virBuffer buf; + virBuffer buf = VIR_BUFFER_INITIALIZER; int hvm = 0, bootloader = 0, vfb = 0; int domid = -1; int max_mem, cur_mem; @@ -1647,11 +1632,6 @@ xend_parse_sexp_desc(virConnectPtr conn, /* ERROR */ return (NULL); } - buf.content = malloc(4000); - if (buf.content == NULL) - return (NULL); - buf.size = 4000; - buf.use = 0; tmp = sexpr_node(root, "domain/domid"); if (tmp == NULL && xendConfigVersion < 3) { /* Old XenD, domid was mandatory */ @@ -2097,11 +2077,15 @@ xend_parse_sexp_desc(virConnectPtr conn, virBufferAddLit(&buf, " </devices>\n"); virBufferAddLit(&buf, "</domain>\n"); - buf.content[buf.use] = 0; - return (buf.content); + if (virBufferError(&buf)) { + virXendError(conn, VIR_ERR_NO_MEMORY, _("allocate buffer")); + return NULL; + } + + return virBufferContentAndReset(&buf); error: - free(buf.content); + free(virBufferContentAndReset(&buf)); return (NULL); } Index: src/xm_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xm_internal.c,v retrieving revision 1.71 diff -u -p -r1.71 xm_internal.c --- src/xm_internal.c 26 Apr 2008 14:22:02 -0000 1.71 +++ src/xm_internal.c 26 Apr 2008 16:24:54 -0000 @@ -579,8 +579,7 @@ int xenXMDomainGetInfo(virDomainPtr doma * domain, suitable for later feeding for virDomainCreateLinux */ char *xenXMDomainFormatXML(virConnectPtr conn, virConfPtr conf) { - virBufferPtr buf; - char *xml; + virBuffer buf = VIR_BUFFER_INITIALIZER; const char *name; unsigned char uuid[VIR_UUID_BUFLEN]; char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -602,12 +601,10 @@ char *xenXMDomainFormatXML(virConnectPtr if (xenXMConfigGetUUID(conf, "uuid", uuid) < 0) return (NULL); - buf = virBufferNew(4096); - - virBufferAddLit(buf, "<domain type='xen'>\n"); - virBufferVSprintf(buf, " <name>%s</name>\n", name); + virBufferAddLit(&buf, "<domain type='xen'>\n"); + virBufferVSprintf(&buf, " <name>%s</name>\n", name); virUUIDFormat(uuid, uuidstr); - virBufferVSprintf(buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferVSprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); if ((xenXMConfigGetString(conf, "builder", &str) == 0) && !strcmp(str, "hvm")) @@ -615,10 +612,10 @@ char *xenXMDomainFormatXML(virConnectPtr if (hvm) { const char *boot; - virBufferAddLit(buf, " <os>\n"); - virBufferAddLit(buf, " <type>hvm</type>\n"); + virBufferAddLit(&buf, " <os>\n"); + virBufferAddLit(&buf, " <type>hvm</type>\n"); if (xenXMConfigGetString(conf, "kernel", &str) == 0) - virBufferVSprintf(buf, " <loader>%s</loader>\n", str); + virBufferVSprintf(&buf, " <loader>%s</loader>\n", str); if (xenXMConfigGetString(conf, "boot", &boot) < 0) boot = "c"; @@ -637,90 +634,90 @@ char *xenXMDomainFormatXML(virConnectPtr dev = "hd"; break; } - virBufferVSprintf(buf, " <boot dev='%s'/>\n", dev); + virBufferVSprintf(&buf, " <boot dev='%s'/>\n", dev); boot++; } - virBufferAddLit(buf, " </os>\n"); + virBufferAddLit(&buf, " </os>\n"); } else { if (xenXMConfigGetString(conf, "bootloader", &str) == 0) - virBufferVSprintf(buf, " <bootloader>%s</bootloader>\n", str); + virBufferVSprintf(&buf, " <bootloader>%s</bootloader>\n", str); if (xenXMConfigGetString(conf, "bootargs", &str) == 0) - virBufferEscapeString(buf, " <bootloader_args>%s</bootloader_args>\n", str); + virBufferEscapeString(&buf, " <bootloader_args>%s</bootloader_args>\n", str); if (xenXMConfigGetString(conf, "kernel", &str) == 0) { - virBufferAddLit(buf, " <os>\n"); - virBufferAddLit(buf, " <type>linux</type>\n"); - virBufferVSprintf(buf, " <kernel>%s</kernel>\n", str); + virBufferAddLit(&buf, " <os>\n"); + virBufferAddLit(&buf, " <type>linux</type>\n"); + virBufferVSprintf(&buf, " <kernel>%s</kernel>\n", str); if (xenXMConfigGetString(conf, "ramdisk", &str) == 0) - virBufferVSprintf(buf, " <initrd>%s</initrd>\n", str); + virBufferVSprintf(&buf, " <initrd>%s</initrd>\n", str); if (xenXMConfigGetString(conf, "extra", &str) == 0) - virBufferEscapeString(buf, " <cmdline>%s</cmdline>\n", str); - virBufferAddLit(buf, " </os>\n"); + virBufferEscapeString(&buf, " <cmdline>%s</cmdline>\n", str); + virBufferAddLit(&buf, " </os>\n"); } } if (xenXMConfigGetInt(conf, "memory", &val) < 0) val = MIN_XEN_GUEST_SIZE * 2; - virBufferVSprintf(buf, " <currentMemory>%ld</currentMemory>\n", + virBufferVSprintf(&buf, " <currentMemory>%ld</currentMemory>\n", val * 1024); if (xenXMConfigGetInt(conf, "maxmem", &val) < 0) if (xenXMConfigGetInt(conf, "memory", &val) < 0) val = MIN_XEN_GUEST_SIZE * 2; - virBufferVSprintf(buf, " <memory>%ld</memory>\n", val * 1024); + virBufferVSprintf(&buf, " <memory>%ld</memory>\n", val * 1024); - virBufferAddLit(buf, " <vcpu"); + virBufferAddLit(&buf, " <vcpu"); if (xenXMConfigGetString(conf, "cpus", &str) == 0) { char *ranges; ranges = virConvertCpuSet(conn, str, 0); if (ranges != NULL) { - virBufferVSprintf(buf, " cpuset='%s'", ranges); + virBufferVSprintf(&buf, " cpuset='%s'", ranges); free(ranges); } else - virBufferVSprintf(buf, " cpuset='%s'", str); + virBufferVSprintf(&buf, " cpuset='%s'", str); } if (xenXMConfigGetInt(conf, "vcpus", &val) < 0) val = 1; - virBufferVSprintf(buf, ">%ld</vcpu>\n", val); + virBufferVSprintf(&buf, ">%ld</vcpu>\n", val); if (xenXMConfigGetString(conf, "on_poweroff", &str) < 0) str = "destroy"; - virBufferVSprintf(buf, " <on_poweroff>%s</on_poweroff>\n", str); + virBufferVSprintf(&buf, " <on_poweroff>%s</on_poweroff>\n", str); if (xenXMConfigGetString(conf, "on_reboot", &str) < 0) str = "restart"; - virBufferVSprintf(buf, " <on_reboot>%s</on_reboot>\n", str); + virBufferVSprintf(&buf, " <on_reboot>%s</on_reboot>\n", str); if (xenXMConfigGetString(conf, "on_crash", &str) < 0) str = "restart"; - virBufferVSprintf(buf, " <on_crash>%s</on_crash>\n", str); + virBufferVSprintf(&buf, " <on_crash>%s</on_crash>\n", str); if (hvm) { - virBufferAddLit(buf, " <features>\n"); + virBufferAddLit(&buf, " <features>\n"); if (xenXMConfigGetInt(conf, "pae", &val) == 0 && val) - virBufferAddLit(buf, " <pae/>\n"); + virBufferAddLit(&buf, " <pae/>\n"); if (xenXMConfigGetInt(conf, "acpi", &val) == 0 && val) - virBufferAddLit(buf, " <acpi/>\n"); + virBufferAddLit(&buf, " <acpi/>\n"); if (xenXMConfigGetInt(conf, "apic", &val) == 0 && val) - virBufferAddLit(buf, " <apic/>\n"); - virBufferAddLit(buf, " </features>\n"); + virBufferAddLit(&buf, " <apic/>\n"); + virBufferAddLit(&buf, " </features>\n"); if (xenXMConfigGetInt(conf, "localtime", &val) < 0) val = 0; - virBufferVSprintf(buf, " <clock offset='%s'/>\n", val ? "localtime" : "utc"); + virBufferVSprintf(&buf, " <clock offset='%s'/>\n", val ? "localtime" : "utc"); } - virBufferAddLit(buf, " <devices>\n"); + virBufferAddLit(&buf, " <devices>\n"); if (hvm) { if (xenXMConfigGetString(conf, "device_model", &str) == 0) - virBufferVSprintf(buf, " <emulator>%s</emulator>\n", str); + virBufferVSprintf(&buf, " <emulator>%s</emulator>\n", str); } list = virConfGetValue(conf, "disk"); @@ -808,23 +805,23 @@ char *xenXMDomainFormatXML(virConnectPtr tmp[0] = '\0'; } - virBufferVSprintf(buf, " <disk type='%s' device='%s'>\n", + virBufferVSprintf(&buf, " <disk type='%s' device='%s'>\n", block ? "block" : "file", cdrom ? "cdrom" : "disk"); if (drvType[0]) - virBufferVSprintf(buf, " <driver name='%s' type='%s'/>\n", drvName, drvType); + virBufferVSprintf(&buf, " <driver name='%s' type='%s'/>\n", drvName, drvType); else - virBufferVSprintf(buf, " <driver name='%s'/>\n", drvName); + virBufferVSprintf(&buf, " <driver name='%s'/>\n", drvName); if (src[0]) - virBufferVSprintf(buf, " <source %s='%s'/>\n", block ? "dev" : "file", src); - virBufferVSprintf(buf, " <target dev='%s'/>\n", dev); + virBufferVSprintf(&buf, " <source %s='%s'/>\n", block ? "dev" : "file", src); + virBufferVSprintf(&buf, " <target dev='%s'/>\n", dev); if (!strcmp(head, "r") || !strcmp(head, "ro")) - virBufferAddLit(buf, " <readonly/>\n"); + virBufferAddLit(&buf, " <readonly/>\n"); else if ((!strcmp(head, "w!")) || (!strcmp(head, "!"))) - virBufferAddLit(buf, " <shareable/>\n"); - virBufferAddLit(buf, " </disk>\n"); + virBufferAddLit(&buf, " <shareable/>\n"); + virBufferAddLit(&buf, " </disk>\n"); skipdisk: list = list->next; @@ -833,12 +830,12 @@ char *xenXMDomainFormatXML(virConnectPtr if (hvm && priv->xendConfigVersion == 1) { if (xenXMConfigGetString(conf, "cdrom", &str) == 0) { - virBufferAddLit(buf, " <disk type='file' device='cdrom'>\n"); - virBufferAddLit(buf, " <driver name='file'/>\n"); - virBufferVSprintf(buf, " <source file='%s'/>\n", str); - virBufferAddLit(buf, " <target dev='hdc'/>\n"); - virBufferAddLit(buf, " <readonly/>\n"); - virBufferAddLit(buf, " </disk>\n"); + virBufferAddLit(&buf, " <disk type='file' device='cdrom'>\n"); + virBufferAddLit(&buf, " <driver name='file'/>\n"); + virBufferVSprintf(&buf, " <source file='%s'/>\n", str); + virBufferAddLit(&buf, " <target dev='hdc'/>\n"); + virBufferAddLit(&buf, " <readonly/>\n"); + virBufferAddLit(&buf, " </disk>\n"); } } @@ -909,16 +906,16 @@ char *xenXMDomainFormatXML(virConnectPtr type = 1; } - virBufferAddLit(buf, " <interface type='bridge'>\n"); + virBufferAddLit(&buf, " <interface type='bridge'>\n"); if (mac[0]) - virBufferVSprintf(buf, " <mac address='%s'/>\n", mac); + virBufferVSprintf(&buf, " <mac address='%s'/>\n", mac); if (type == 1 && bridge[0]) - virBufferVSprintf(buf, " <source bridge='%s'/>\n", bridge); + virBufferVSprintf(&buf, " <source bridge='%s'/>\n", bridge); if (script[0]) - virBufferVSprintf(buf, " <script path='%s'/>\n", script); + virBufferVSprintf(&buf, " <script path='%s'/>\n", script); if (ip[0]) - virBufferVSprintf(buf, " <ip address='%s'/>\n", ip); - virBufferAddLit(buf, " </interface>\n"); + virBufferVSprintf(&buf, " <ip address='%s'/>\n", ip); + virBufferAddLit(&buf, " </interface>\n"); skipnic: list = list->next; @@ -928,9 +925,9 @@ char *xenXMDomainFormatXML(virConnectPtr if (hvm) { if (xenXMConfigGetString(conf, "usbdevice", &str) == 0 && str) { if (!strcmp(str, "tablet")) - virBufferAddLit(buf, " <input type='tablet' bus='usb'/>\n"); + virBufferAddLit(&buf, " <input type='tablet' bus='usb'/>\n"); else if (!strcmp(str, "mouse")) - virBufferAddLit(buf, " <input type='mouse' bus='usb'/>\n"); + virBufferAddLit(&buf, " <input type='mouse' bus='usb'/>\n"); /* Ignore else branch - probably some other non-input device we don't support in libvirt yet */ } @@ -1003,54 +1000,56 @@ char *xenXMDomainFormatXML(virConnectPtr } if (vnc || sdl) { - virBufferVSprintf(buf, " <input type='mouse' bus='%s'/>\n", hvm ? "ps2":"xen"); + virBufferVSprintf(&buf, " <input type='mouse' bus='%s'/>\n", hvm ? "ps2":"xen"); } if (vnc) { - virBufferVSprintf(buf, + virBufferVSprintf(&buf, " <graphics type='vnc' port='%ld'", (vncunused ? -1 : 5900+vncdisplay)); if (vnclisten) { - virBufferVSprintf(buf, " listen='%s'", vnclisten); + virBufferVSprintf(&buf, " listen='%s'", vnclisten); } if (vncpasswd) { - virBufferVSprintf(buf, " passwd='%s'", vncpasswd); + virBufferVSprintf(&buf, " passwd='%s'", vncpasswd); } if (keymap) { - virBufferVSprintf(buf, " keymap='%s'", keymap); + virBufferVSprintf(&buf, " keymap='%s'", keymap); } - virBufferAddLit(buf, "/>\n"); + virBufferAddLit(&buf, "/>\n"); } if (sdl) { - virBufferAddLit(buf, " <graphics type='sdl'/>\n"); + virBufferAddLit(&buf, " <graphics type='sdl'/>\n"); } if (hvm) { if (xenXMConfigGetString(conf, "parallel", &str) == 0) { if (STRNEQ(str, "none")) - xend_parse_sexp_desc_char(conn, buf, "parallel", 0, str, NULL); + xend_parse_sexp_desc_char(conn, &buf, "parallel", 0, str, NULL); } if (xenXMConfigGetString(conf, "serial", &str) == 0) { if (STRNEQ(str, "none")) { - xend_parse_sexp_desc_char(conn, buf, "serial", 0, str, NULL); + xend_parse_sexp_desc_char(conn, &buf, "serial", 0, str, NULL); /* Add back-compat console tag for primary console */ - xend_parse_sexp_desc_char(conn, buf, "console", 0, str, NULL); + xend_parse_sexp_desc_char(conn, &buf, "console", 0, str, NULL); } } } else { /* Paravirt implicitly always has a single console */ - virBufferAddLit(buf, " <console type='pty'>\n"); - virBufferAddLit(buf, " <target port='0'/>\n"); - virBufferAddLit(buf, " </console>\n"); + virBufferAddLit(&buf, " <console type='pty'>\n"); + virBufferAddLit(&buf, " <target port='0'/>\n"); + virBufferAddLit(&buf, " </console>\n"); } - virBufferAddLit(buf, " </devices>\n"); + virBufferAddLit(&buf, " </devices>\n"); + + virBufferAddLit(&buf, "</domain>\n"); - virBufferAddLit(buf, "</domain>\n"); + if (virBufferError(&buf)) { + xenXMError(conn, VIR_ERR_NO_MEMORY, _("allocate buffer")); + return NULL; + } - xml = buf->content; - buf->content = NULL; - virBufferFree(buf); - return (xml); + return virBufferContentAndReset(&buf); } @@ -1254,7 +1253,7 @@ int xenXMDomainPinVcpu(virDomainPtr doma { const char *filename; xenXMConfCachePtr entry; - virBufferPtr mapbuf; + virBuffer mapbuf = VIR_BUFFER_INITIALIZER; char *mapstr = NULL; char *ranges = NULL; int i, j, n, comma = 0; @@ -1288,33 +1287,24 @@ int xenXMDomainPinVcpu(virDomainPtr doma } /* from bit map, build character string of mapped CPU numbers */ - mapbuf = virBufferNew (16); - if (mapbuf == NULL) { - xenXMError (domain->conn, VIR_ERR_NO_MEMORY, __FUNCTION__); - return -1; - } for (i = 0; i < maplen; i++) for (j = 0; j < 8; j++) if ((cpumap[i] & (1 << j))) { n = i*8 + j; - if (comma) { - if (virBufferAddLit (mapbuf, ",") == -1) { - xenXMError (domain->conn, VIR_ERR_NO_MEMORY, __FUNCTION__); - virBufferFree (mapbuf); - return -1; - } - } + if (comma) + virBufferAddLit (&mapbuf, ","); comma = 1; - if (virBufferVSprintf (mapbuf, "%d", n) == -1) { - xenXMError (domain->conn, VIR_ERR_NO_MEMORY, __FUNCTION__); - virBufferFree (mapbuf); - return -1; - } + virBufferVSprintf (&mapbuf, "%d", n); } - mapstr = virBufferContentAndFree (mapbuf); + if (virBufferError(&mapbuf)) { + xenXMError(domain->conn, VIR_ERR_NO_MEMORY, _("allocate buffer")); + return -1; + } + + mapstr = virBufferContentAndReset(&mapbuf); /* convert the mapstr to a range based string */ ranges = virConvertCpuSet(domain->conn, mapstr, 0); Index: src/xml.c =================================================================== RCS file: /data/cvs/libvirt/src/xml.c,v retrieving revision 1.118 diff -u -p -r1.118 xml.c --- src/xml.c 26 Apr 2008 14:22:02 -0000 1.118 +++ src/xml.c 26 Apr 2008 16:24:55 -0000 @@ -102,19 +102,13 @@ parseCpuNumber(const char **str, int max char * virSaveCpuSet(virConnectPtr conn, char *cpuset, int maxcpu) { - virBufferPtr buf; - char *ret; + virBuffer buf = VIR_BUFFER_INITIALIZER; int start, cur; int first = 1; if ((cpuset == NULL) || (maxcpu <= 0) || (maxcpu > 100000)) return (NULL); - buf = virBufferNew(1000); - if (buf == NULL) { - virXMLError(conn, VIR_ERR_NO_MEMORY, _("allocate buffer"), 1000); - return (NULL); - } cur = 0; start = -1; while (cur < maxcpu) { @@ -123,27 +117,32 @@ virSaveCpuSet(virConnectPtr conn, char * start = cur; } else if (start != -1) { if (!first) - virBufferAddLit(buf, ","); + virBufferAddLit(&buf, ","); else first = 0; if (cur == start + 1) - virBufferVSprintf(buf, "%d", start); + virBufferVSprintf(&buf, "%d", start); else - virBufferVSprintf(buf, "%d-%d", start, cur - 1); + virBufferVSprintf(&buf, "%d-%d", start, cur - 1); start = -1; } cur++; } if (start != -1) { if (!first) - virBufferAddLit(buf, ","); + virBufferAddLit(&buf, ","); if (maxcpu == start + 1) - virBufferVSprintf(buf, "%d", start); + virBufferVSprintf(&buf, "%d", start); else - virBufferVSprintf(buf, "%d-%d", start, maxcpu - 1); + virBufferVSprintf(&buf, "%d-%d", start, maxcpu - 1); } - ret = virBufferContentAndFree(buf); - return (ret); + + if (virBufferError(&buf)) { + virXMLError(conn, VIR_ERR_NO_MEMORY, _("allocate buffer"), 1000); + return NULL; + } + + return virBufferContentAndReset(&buf); } /** @@ -1054,11 +1053,9 @@ virDomainParseXMLOSDescHVM(virConnectPtr char scratch[PATH_MAX]; if (virDomainParseXMLOSDescHVMChar(conn, scratch, sizeof(scratch), cur) < 0) goto error; - if (virBufferVSprintf(buf, "(parallel %s)", scratch) < 0) - goto no_memory; + virBufferVSprintf(buf, "(parallel %s)", scratch); } else { - if (virBufferAddLit(buf, "(parallel none)") < 0) - goto no_memory; + virBufferAddLit(buf, "(parallel none)"); } cur = virXPathNode("/domain/devices/serial[1]", ctxt); @@ -1066,8 +1063,7 @@ virDomainParseXMLOSDescHVM(virConnectPtr char scratch[PATH_MAX]; if (virDomainParseXMLOSDescHVMChar(conn, scratch, sizeof(scratch), cur) < 0) goto error; - if (virBufferVSprintf(buf, "(serial %s)", scratch) < 0) - goto no_memory; + virBufferVSprintf(buf, "(serial %s)", scratch); } else { res = virXPathBoolean("count(domain/devices/console) > 0", ctxt); if (res < 0) { @@ -1075,27 +1071,20 @@ virDomainParseXMLOSDescHVM(virConnectPtr goto error; } if (res) { - if (virBufferAddLit(buf, "(serial pty)") < 0) - goto no_memory; + virBufferAddLit(buf, "(serial pty)"); } else { - if (virBufferAddLit(buf, "(serial none)") < 0) - goto no_memory; + virBufferAddLit(buf, "(serial none)"); } } str = virXPathString("string(/domain/clock/@offset)", ctxt); - if (str != NULL && !strcmp(str, "localtime")) { - if (virBufferAddLit(buf, "(localtime 1)") < 0) - goto no_memory; + if (str != NULL && STREQ(str, "localtime")) { + virBufferAddLit(buf, "(localtime 1)"); } free(str); return (0); -no_memory: - virXMLError(conn, VIR_ERR_XML_ERROR, - _("cannot allocate memory for buffer"), 0); - error: free(nodes); return (-1); @@ -1509,7 +1498,7 @@ virDomainParseXMLDesc(virConnectPtr conn xmlDocPtr xml = NULL; xmlNodePtr node; char *nam = NULL; - virBuffer buf; + virBuffer buf = VIR_BUFFER_INITIALIZER; xmlChar *prop; xmlParserCtxtPtr pctxt; xmlXPathContextPtr ctxt = NULL; @@ -1525,11 +1514,6 @@ virDomainParseXMLDesc(virConnectPtr conn if (name != NULL) *name = NULL; - buf.content = malloc(1000); - if (buf.content == NULL) - return (NULL); - buf.size = 1000; - buf.use = 0; pctxt = xmlNewParserCtxt(); if ((pctxt == NULL) || (pctxt->sax == NULL)) { @@ -1787,7 +1771,6 @@ virDomainParseXMLDesc(virConnectPtr conn virBufferAddLit(&buf, ")"); /* closes (vm */ - buf.content[buf.use] = 0; xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); @@ -1798,7 +1781,12 @@ virDomainParseXMLDesc(virConnectPtr conn else free(nam); - return (buf.content); + if (virBufferError(&buf)) { + virXMLError(conn, VIR_ERR_NO_MEMORY, _("allocate buffer"), 0); + return NULL; + } + + return virBufferContentAndReset(&buf); error: free(nam); @@ -1809,7 +1797,7 @@ virDomainParseXMLDesc(virConnectPtr conn xmlFreeDoc(xml); if (pctxt != NULL) xmlFreeParserCtxt(pctxt); - free(buf.content); + free(virBufferContentAndReset(&buf)); return (NULL); } @@ -1834,14 +1822,8 @@ virParseXMLDevice(virConnectPtr conn, co { xmlDocPtr xml = NULL; xmlNodePtr node; - virBuffer buf; + virBuffer buf = VIR_BUFFER_INITIALIZER; - buf.content = malloc(1000); - if (buf.content == NULL) - return (NULL); - buf.size = 1000; - buf.use = 0; - buf.content[0] = 0; xml = xmlReadDoc((const xmlChar *) xmldesc, "device.xml", NULL, XML_PARSE_NOENT | XML_PARSE_NONET | XML_PARSE_NOERROR | XML_PARSE_NOWARNING); @@ -1856,9 +1838,6 @@ virParseXMLDevice(virConnectPtr conn, co if (virDomainParseXMLDiskDesc(conn, node, &buf, hvm, xendConfigVersion) != 0) goto error; - /* SXP is not created when device is "floppy". */ - else if (buf.use == 0) - goto error; } else if (xmlStrEqual(node->name, BAD_CAST "interface")) { if (virDomainParseXMLIfDesc(conn, node, &buf, hvm, xendConfigVersion) != 0) @@ -1867,14 +1846,20 @@ virParseXMLDevice(virConnectPtr conn, co virXMLError(conn, VIR_ERR_XML_ERROR, (const char *) node->name, 0); goto error; } - cleanup: - if (xml != NULL) - xmlFreeDoc(xml); - return buf.content; + + xmlFreeDoc(xml); + + if (virBufferError(&buf)) { + virXMLError(conn, VIR_ERR_NO_MEMORY, _("allocate buffer"), 0); + return NULL; + } + + return virBufferContentAndReset(&buf); + error: - free(buf.content); - buf.content = NULL; - goto cleanup; + free(virBufferContentAndReset(&buf)); + xmlFreeDoc(xml); + return NULL; } Index: src/xmlrpc.c =================================================================== RCS file: /data/cvs/libvirt/src/xmlrpc.c,v retrieving revision 1.13 diff -u -p -r1.13 xmlrpc.c --- src/xmlrpc.c 10 Apr 2008 16:54:54 -0000 1.13 +++ src/xmlrpc.c 26 Apr 2008 16:24:56 -0000 @@ -363,14 +363,12 @@ void xmlRpcValueMarshal(xmlRpcValuePtr v virBufferStrcat(buf, "</value>\n", NULL); } -virBufferPtr xmlRpcMarshalRequest(const char *request, - int argc, xmlRpcValuePtr *argv) +void xmlRpcMarshalRequest(const char *request, + virBufferPtr buf, + int argc, xmlRpcValuePtr *argv) { - virBufferPtr buf; int i; - buf = virBufferNew(1024); - virBufferStrcat(buf, "<?xml version=\"1.0\"?>\n" "<methodCall>\n" @@ -386,7 +384,6 @@ virBufferPtr xmlRpcMarshalRequest(const virBufferStrcat(buf, " </params>\n" "</methodCall>\n", NULL); - return buf; } xmlRpcValuePtr xmlRpcUnmarshalResponse(xmlNodePtr node, bool *is_fault) @@ -564,13 +561,14 @@ int xmlRpcCall(xmlRpcContextPtr context, va_list ap; int argc; xmlRpcValuePtr *argv; - virBufferPtr buf; + virBuffer buf = VIR_BUFFER_INITIALIZER; char *ret; xmlDocPtr xml; xmlNodePtr node; bool fault; xmlRpcValuePtr value; void *retval = NULL; + char *content; va_start(ap, fmt); @@ -582,16 +580,16 @@ int xmlRpcCall(xmlRpcContextPtr context, va_end(ap); - buf = xmlRpcMarshalRequest(method, argc, argv); + xmlRpcMarshalRequest(method, &buf, argc, argv); xmlRpcArgvFree(argc, argv); - if (!buf) + if (virBufferError(&buf)) return -1; - ret = xmlRpcCallRaw(context->uri, buf->content); - - virBufferFree(buf); + content = virBufferContentAndReset(&buf); + ret = xmlRpcCallRaw(context->uri, content); + free(content); if (!ret) return -1; Index: src/xmlrpc.h =================================================================== RCS file: /data/cvs/libvirt/src/xmlrpc.h,v retrieving revision 1.4 diff -u -p -r1.4 xmlrpc.h --- src/xmlrpc.h 10 Apr 2008 16:54:54 -0000 1.4 +++ src/xmlrpc.h 26 Apr 2008 16:24:56 -0000 @@ -89,8 +89,9 @@ struct _xmlRpcContext; xmlRpcValuePtr *xmlRpcArgvNew(const char *fmt, va_list ap, int *argc); void xmlRpcArgvFree(int argc, xmlRpcValuePtr *argv); -virBufferPtr xmlRpcMarshalRequest(const char *request, - int argc, xmlRpcValuePtr *argv); +void xmlRpcMarshalRequest(const char *request, + virBufferPtr buf, + int argc, xmlRpcValuePtr *argv); xmlRpcValuePtr xmlRpcUnmarshalResponse(xmlNodePtr node, bool *is_fault); Index: tests/xmlrpctest.c =================================================================== RCS file: /data/cvs/libvirt/tests/xmlrpctest.c,v retrieving revision 1.11 diff -u -p -r1.11 xmlrpctest.c --- tests/xmlrpctest.c 10 Apr 2008 16:54:54 -0000 1.11 +++ tests/xmlrpctest.c 26 Apr 2008 16:24:56 -0000 @@ -59,22 +59,20 @@ testMethodPlusDOUBLE(const void *data) return retval==(10.1234+10.1234) ? 0 : -1; } -static virBufferPtr -marshalRequest(const char *fmt, ...) +static void +marshalRequest(virBufferPtr buf, const char *fmt, ...) { int argc; xmlRpcValuePtr *argv; - virBufferPtr buf; va_list ap; va_start(ap, fmt); argv = xmlRpcArgvNew(fmt, ap, &argc); va_end(ap); - buf = xmlRpcMarshalRequest("test", argc, argv); + xmlRpcMarshalRequest("test", buf, argc, argv); xmlRpcArgvFree(argc, argv); - return buf; } static int @@ -132,14 +130,21 @@ testMarshalRequestINT(const void *data) int num = INT_MAX; int ret = 0; int check = data ? *((int *)data) : 0; - virBufferPtr buf = marshalRequest("i", num); + virBuffer buf = VIR_BUFFER_INITIALIZER; + marshalRequest(&buf, "i", num); + char *content; + + if (virBufferError(&buf)) + return -1; + + content = virBufferContentAndReset(&buf); if (check) - ret = checkRequestValue(buf->content, + ret = checkRequestValue(content, "number(/methodCall/params/param[1]/value/int)", XML_RPC_INTEGER, (void *) &num); - virBufferFree(buf); + free(content); return ret; } @@ -149,13 +154,21 @@ testMarshalRequestSTRING(const void *dat const char *str = "This library will be really sexy."; int ret = 0; int check = data ? *((int *)data) : 0; - virBufferPtr buf = marshalRequest("s", str); + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *content; + + marshalRequest(&buf, "s", str); + if (virBufferError(&buf)) + return -1; + + content = virBufferContentAndReset(&buf); if (check) - ret = checkRequestValue(buf->content, + ret = checkRequestValue(content, "string(/methodCall/params/param[1]/value/string)", XML_RPC_STRING, (void *) str); - virBufferFree(buf); + + free(content); return ret; } @@ -165,42 +178,24 @@ testMarshalRequestDOUBLE(const void *dat double num = 123456789.123; int ret = 0; int check = data ? *((int *)data) : 0; - virBufferPtr buf = marshalRequest("f", num); + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *content; + + marshalRequest(&buf, "f", num); + if (virBufferError(&buf)) + return -1; + + content = virBufferContentAndReset(&buf); if (check) - ret = checkRequestValue(buf->content, + ret = checkRequestValue(content, "number(/methodCall/params/param[1]/value/double)", XML_RPC_DOUBLE, (void *) &num); - virBufferFree(buf); + free(content); return ret; } -static int -testBufferStrcat(const void *data ATTRIBUTE_UNUSED) -{ - virBufferPtr buf = virBufferNew(1000*32); /* don't waste time with realloc */ - int i; - - for (i=0; i < 1000; i++) - virBufferStrcat(buf, "My name is ", "libvirt", ".\n", NULL); - - virBufferFree(buf); - return 0; -} - -static int -testBufferVSprintf(const void *data ATTRIBUTE_UNUSED) -{ - virBufferPtr buf = virBufferNew(1000*32); /* don't waste time with realloc */ - int i; - - for (i=0; i < 1000; i++) - virBufferVSprintf(buf, "My name is %s.\n", "libvirt"); - - virBufferFree(buf); - return 0; -} int main(int argc, char **argv) @@ -263,13 +258,7 @@ main(int argc, char **argv) NLOOPS, testMarshalRequestSTRING, NULL) != 0) ret = -1; - if (virtTestRun("Buffer: strcat", NLOOPS, testBufferStrcat, NULL) != 0) - ret = -1; - if (virtTestRun("Buffer: sprintf", NLOOPS, testBufferVSprintf, NULL) != 0) - ret = -1; - - - exit(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); + exit(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } -- |: 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 :|

2 6

[Libvir] Integrating NetApp/Solaris iSCSI highlevel facilities
by Stefan de Konink 28 Apr '08

28 Apr '08

Hi All, I have just finished some scripts that allows 'native Xen' to use some facilities of NetApp. Basically using a vfiler, and a q-tree Xen can be provisioned. Allowing: netapp://customer/disk Some scripts can be found here: http://kinkrsoftware.nl/contrib/xen/ I wonder if people are already working on the libvirt storage side to support 'remote' highlevel filesystems. For example ZFS or NetApp storage machines could benefit from an easy interface. Currently I'm building these 'block' scripts because they give straightforward facities to my setup. But if anyone wants to team up to get this all inside some more abstract libvirt setup I would happy to join. Stefan

2 2

[Libvir] Fedora spec file changes
by Mark McLoughlin 28 Apr '08

28 Apr '08

Hey, First thing - libvirt.spec has previously matched the spec file we've used in Fedora for building packages, but now it's quite a bit out of sync. My question is whether libvirt.spec should be upstream at all: 1) Having the two copies is confusing - which is canonical? 2) Keeping the two copies in sync is time consuming, especially if they're not going to be exactly identical. 3) It's not clear that it's useful to have it upstream at all - i.e. is it useful anywhere but Fedora? Are iscsi-initiator-utils or selinux-devel valid RPM names on any other distro? 4) If we're to keep libvirt.spec in CVS purely as a convenience for Fedora users wanting to build the latest tarball, then we should be ensuring that libvirt.spec is usable for the tarball *before* releasing it, which is a pain. Personally, I think we should remove it from upstream libvirt. Finally, here's some changes to how arch conditionals are handed in the spec file - what prompted this was the way we were enabling with_proxy even when with_xen was disabled, causing the proxy to be in the files section even when it wasn't built. Please give it a look over before I built in dist-f10. Cheers, Mark.

6 10