
Matthias Bolte wrote:
2010/1/18 Jim Meyering <jim@meyering.net>:
At first I was going to call virDomainDeviceDefFree only "if (dev)", but saw that it handles a NULL "dev" just fine, so it's better to skip the test altogether, just as we do for many other free-like functions.
From ea8511d709492f5cdc152a1eaccbccd05f036648 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 18 Jan 2010 16:55:36 +0100 Subject: [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer
* src/qemu/qemu_driver.c (qemudDomainAttachDevice): Don't leak "dev". ...
NACK. This will probably result in a segfault because you are freeing memory that is still in use.
Yes the toplevel dev leaks here, but for example qemudDomainAttachNetDevice some lines above takes parts from the dev struct an assigns them to other structs _without_ copying them.
Thanks. That is nastily unintuitive and sounds a lot like a bug. I hope it's on the list of things to be fixed by Dan's patch.
I found this leak some time ago too, but gave up on fixing it as I noticed how entangled this code is.