On 12/10/2010 12:58 PM, Laine Stump wrote:
On 12/10/2010 02:18 PM, Eric Blake wrote:
* src/util/command.h (virCommandAddArgBuffer) (virCommandAddEnvBuffer): New prototypes. * src/util/command.c (virCommandAddArgBuffer) (virCommandAddEnvBuffer): Implement them. * src/libvirt_private.syms (command.h): Export them. * src/qemu/qemu_conf.c (qemudBuildCommandLine): Use them, plugging a memory leak on rbd_hosts in the process. ---
BTW, I'm really loving the ability to just forget about error checking until after all the args are added. It really cuts down on the lines of code, as well as making it easier to follow what's really happening.
Me too!
src/libvirt_private.syms | 2 ++ src/qemu/qemu_conf.c | 46 +++++++++------------------------------------- src/util/command.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/util/command.h | 17 +++++++++++++++++ 4 files changed, 71 insertions(+), 37 deletions(-)
ACK. The new APIs and their uses all look fine to me.
Actually, I noticed a minor problem. I squashed 3 and 4 together, then squashed this in, then pushed: diff --git i/src/util/command.c w/src/util/command.c index 90c0d3a..f9d475e 100644 --- i/src/util/command.c +++ w/src/util/command.c @@ -316,13 +316,16 @@ virCommandAddEnvString(virCommandPtr cmd, const char *str) /* * Convert a buffer containing preformatted name=value into an - * environment variable of the child + * environment variable of the child. + * Correctly transfers memory errors or contents from buf to cmd. */ void virCommandAddEnvBuffer(virCommandPtr cmd, virBufferPtr buf) { - if (!cmd || cmd->has_error) + if (!cmd || cmd->has_error) { + virBufferFreeAndReset(buf); return; + } /* env plus trailing NULL. */ if (virBufferError(buf) || @@ -403,13 +406,16 @@ virCommandAddArg(virCommandPtr cmd, const char *val) /* - * Convert a buffer into a command line argument to the child + * Convert a buffer into a command line argument to the child. + * Correctly transfers memory errors or contents from buf to cmd. */ void virCommandAddArgBuffer(virCommandPtr cmd, virBufferPtr buf) { - if (!cmd || cmd->has_error) + if (!cmd || cmd->has_error) { + virBufferFreeAndReset(buf); return; + } /* Arg plus trailing NULL. */ if (virBufferError(buf) || -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org