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. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list