Re: [PATCH 4/4] command: ease use with virBuffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]