Re: [libvirt] [PATCH 4/4] command: ease use with virBuffer
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.
---
Transferring malloc'd contents from one struct to another can be
tricky; hopefully this is easy enough to understand and hard enough to
abuse to warrant the syntactic shorthand that it provides to
qemu_conf. It helps that this is a transfer from one robust wrapper
to another; since we do NOT want something more direct, like:
/* Add malloc'd str to cmd, transferring ownership of who should free it */
void virCommandTransferArg(virCommandPtr cmd, char *str);
because that just invites bugs from freeing in the wrong place.
Well, you *could* make it less error_prone by having it be a macro which
NULLed out the original pointer as a part of the transfer. That would
still leave the possibility of people sending literal strings, or
buffers allocated on the stack, as str, leading to virCommand attempting
to free something that wasn't malloc'ed, so it's just as well to not
even give them the chance ;-)
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.
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.