On 15.07.2014 14:58, Ján Tomko wrote:
On 07/15/2014 02:38 PM, Michal Privoznik wrote:
There are just two places where we rely on the fact that VIR_FREE()
macro is without any side effects. In the future, this property of the
macro is going to change, so we need the code to be adjusted to deal
with argument passed to VIR_FREE() being evaluated multiple times.
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/conf/network_conf.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
NACK, this completely removes the side effect. You need to adjust the callers
for that.
Well, the arrays that n<elems> refer to are freed immediately, but I
agree that we should keep the code clean.
IMO we should set n<elems> to 0 in all *Clear functions, not just
virNetworkDNSHostDefClear, after which NULL might be dereferenced based on the
n<elems> variable.
I don't see how that could be possible with current code. The
virNetworkDNSHostDefClear is called only on cleanup paths where
accessing def->names or def->forwarders is not possible anymore.
Jan
Okay, consider this squashed in:
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d60a60e..dcb521f 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -184,6 +184,7 @@ virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def)
for (i = 0; i < def->nnames; i++)
VIR_FREE(def->names[i]);
+ def->nnames = 0;
VIR_FREE(def->names);
}
@@ -204,6 +205,7 @@ virNetworkDNSDefClear(virNetworkDNSDefPtr def)
if (def->forwarders) {
for (i = 0; i < def->nfwds; i++)
VIR_FREE(def->forwarders[i]);
+ def->nfwds = 0;
VIR_FREE(def->forwarders);
}
if (def->txts) {
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list