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@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list