[libvirt] [PATCH 1/2] conf: Don't leak 'primary' video property on error
It's only freed on normal returns from virDomainVideoDefParseXML, but not when erroring out. --- src/conf/domain_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 10f361c..258ee93 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7662,9 +7662,11 @@ virDomainVideoDefParseXML(const xmlNodePtr node, vram = virXMLPropString(cur, "vram"); heads = virXMLPropString(cur, "heads"); - if ((primary = virXMLPropString(cur, "primary")) != NULL) + if ((primary = virXMLPropString(cur, "primary")) != NULL) { if (STREQ(primary, "yes")) def->primary = 1; + VIR_FREE(primary); + } def->accel = virDomainVideoAccelDefParseXML(cur); } @@ -7728,7 +7730,6 @@ virDomainVideoDefParseXML(const xmlNodePtr node, VIR_FREE(ram); VIR_FREE(vram); VIR_FREE(heads); - VIR_FREE(primary); return def; -- 1.8.1.2
It's only freed on normal returns from virDomainVideoDefParseXML, but not when erroring out. --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 258ee93..0c75838 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7736,6 +7736,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node, error: virDomainVideoDefFree(def); VIR_FREE(type); + VIR_FREE(ram); VIR_FREE(vram); VIR_FREE(heads); return NULL; -- 1.8.1.2
On 02/21/2013 08:49 AM, Christophe Fergeau wrote:
It's only freed on normal returns from virDomainVideoDefParseXML, but not when erroring out. --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+)
ACK. Again, going with a more idiomatic cleanup label that cleans up on both success and error might also work, and might also be part of Peter's series.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 258ee93..0c75838 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7736,6 +7736,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node, error: virDomainVideoDefFree(def); VIR_FREE(type); + VIR_FREE(ram); VIR_FREE(vram); VIR_FREE(heads); return NULL;
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02/21/2013 08:49 AM, Christophe Fergeau wrote:
It's only freed on normal returns from virDomainVideoDefParseXML, but not when erroring out. --- src/conf/domain_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
ACK; your fix is reasonable. A more idiomatic fix might be to coalesce the error and normal exits into a single cleanup: label; in fact, Peter's 16-part series on improving domain_conf.c may have done just that. But as I haven't gotten that far in reviewing his series, and this is a real fix, you can go ahead and push and Peter can deal with any rebase fallout. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Feb 21, 2013 at 01:07:27PM -0700, Eric Blake wrote:
On 02/21/2013 08:49 AM, Christophe Fergeau wrote:
It's only freed on normal returns from virDomainVideoDefParseXML, but not when erroring out. --- src/conf/domain_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
ACK; your fix is reasonable. A more idiomatic fix might be to coalesce the error and normal exits into a single cleanup: label; in fact, Peter's 16-part series on improving domain_conf.c may have done just that. But as I haven't gotten that far in reviewing his series, and this is a real fix, you can go ahead and push and Peter can deal with any rebase fallout.
Thanks, I've pushed the 2 patches now. Christophe
participants (2)
-
Christophe Fergeau -
Eric Blake