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

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.

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