Re: [libvirt] [PATCH] Fix virsh {net,pool}-edit

Cole Robinson <crobinso(a)redhat.com&gt; wrote: Hi Cole, That looks good to me. I confirmed it fixes the bug when using the qemu-based driver: cat <<EOF > net.xml <network> <name>N</name> <ip address="192.168.199.1" netmask="255.255.255.0"></ip> </network> EOF printf '#!/bin/sh\nperl -pi -e "s/199/19/" "$@"\n' > ed; chmod +x ed qemud/libvirtd & EDITOR=$PWD/ed src/virsh 'net-define net.xml; net-edit N' However, using the test driver, my little test exposed a bug: EDITOR=$PWD/ed src/virsh -c test:///default 'net-define net.xml; net-edit N' zsh: segmentation fault That was because the "def = NULL" assignment (to make it so the upcoming "free" is a no-op) is too early, since there are uses of def right after that. And the same bug appears in 3 other nearby functions. Here's the fix I'll commit soon, along with tests based on the above. From: Jim Meyering <meyering(a)redhat.com&gt; Date: Fri, 13 Feb 2009 16:49:50 +0100 Subject: [PATCH] test:///default driver: don't dereference NULL "def" * src/test.c (testNetworkCreate, testNetworkDefine): Don't set "def" to NULL until after the final use. (testStoragePoolCreate, testStoragePoolDefine): Likewise. --- src/test.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test.c b/src/test.c index 0e0ec7c..7e4e64d 100644 --- a/src/test.c +++ b/src/test.c @@ -2054,9 +2054,9 @@ static virNetworkPtr testNetworkCreate(virConnectPtr conn, const char *xml) { goto cleanup; } net->active = 1; - def = NULL; ret = virGetNetwork(conn, def->name, def->uuid); + def = NULL; cleanup: virNetworkDefFree(def); @@ -2081,9 +2081,9 @@ static virNetworkPtr testNetworkDefine(virConnectPtr conn, const char *xml) { goto cleanup; } net->persistent = 1; - def = NULL; ret = virGetNetwork(conn, def->name, def->uuid); + def = NULL; cleanup: virNetworkDefFree(def); @@ -2532,7 +2532,6 @@ testStoragePoolCreate(virConnectPtr conn, if (!(pool = virStoragePoolObjAssignDef(conn, &privconn->pools, def))) { goto cleanup; } - def = NULL; if (testStoragePoolObjSetDefaults(conn, pool) == -1) { virStoragePoolObjRemove(&privconn->pools, pool); @@ -2542,6 +2541,7 @@ testStoragePoolCreate(virConnectPtr conn, pool->active = 1; ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid); + def = NULL; cleanup: virStoragePoolDefFree(def); @@ -2571,7 +2571,6 @@ testStoragePoolDefine(virConnectPtr conn, if (!(pool = virStoragePoolObjAssignDef(conn, &privconn->pools, def))) { goto cleanup; } - def = NULL; if (testStoragePoolObjSetDefaults(conn, pool) == -1) { virStoragePoolObjRemove(&privconn->pools, pool); @@ -2580,6 +2579,7 @@ testStoragePoolDefine(virConnectPtr conn, } ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid); + def = NULL; cleanup: virStoragePoolDefFree(def); -- 1.6.2.rc0.234.g2cc0b3