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