[libvirt] [PATCH] Fix leak of iterator in nwfilter instantiate code

The ebiptablesCreateRuleInstanceIterate creates a virNWFilterVarCombIterPtr instance and iterates over it. Unfortunately in doing so, it discards the original pointer. At the end of the method it will thus effectively do virNWFilterVarCombIterFree(NULL), which means it will leak the iterator. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 57c0476..9dbd3ff 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2865,14 +2865,14 @@ ebiptablesCreateRuleInstanceIterate( virNWFilterRuleInstPtr res) { int rc = 0; - virNWFilterVarCombIterPtr vciter; + virNWFilterVarCombIterPtr vciter, tmp; /* rule->vars holds all the variables names that this rule will access. * iterate over all combinations of the variables' values and instantiate * the filtering rule with each combination. */ - vciter = virNWFilterVarCombIterCreate(vars, - rule->varAccess, rule->nVarAccess); + tmp = vciter = virNWFilterVarCombIterCreate(vars, + rule->varAccess, rule->nVarAccess); if (!vciter) return -1; @@ -2881,12 +2881,12 @@ ebiptablesCreateRuleInstanceIterate( nwfilter, rule, ifname, - vciter, + tmp, res); if (rc < 0) break; - vciter = virNWFilterVarCombIterNext(vciter); - } while (vciter != NULL); + tmp = virNWFilterVarCombIterNext(tmp); + } while (tmp != NULL); virNWFilterVarCombIterFree(vciter); -- 1.8.5.3

On Mon, Mar 17, 2014 at 11:40:39AM +0000, Daniel P. Berrange wrote:
The ebiptablesCreateRuleInstanceIterate creates a virNWFilterVarCombIterPtr instance and iterates over it. Unfortunately in doing so, it discards the original pointer. At the end of the method it will thus effectively do virNWFilterVarCombIterFree(NULL), which means it will leak the iterator.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
Opps, this is wrong. The virNWFilterVarCombIterNext method will bizarely free its input parameter at the end. I'll send a different patch to give this saner semantics. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Mar 17, 2014 at 12:33:24PM +0000, Daniel P. Berrange wrote:
On Mon, Mar 17, 2014 at 11:40:39AM +0000, Daniel P. Berrange wrote:
The ebiptablesCreateRuleInstanceIterate creates a virNWFilterVarCombIterPtr instance and iterates over it. Unfortunately in doing so, it discards the original pointer. At the end of the method it will thus effectively do virNWFilterVarCombIterFree(NULL), which means it will leak the iterator.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
Opps, this is wrong. The virNWFilterVarCombIterNext method will bizarely free its input parameter at the end. I'll send a different patch to give this saner semantics.
Haven't noticed it in the review, sorry, disregard that ACK then :( Martin

On Mon, Mar 17, 2014 at 11:40:39AM +0000, Daniel P. Berrange wrote:
The ebiptablesCreateRuleInstanceIterate creates a virNWFilterVarCombIterPtr instance and iterates over it. Unfortunately in doing so, it discards the original pointer. At the end of the method it will thus effectively do virNWFilterVarCombIterFree(NULL), which means it will leak the iterator.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 57c0476..9dbd3ff 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2865,14 +2865,14 @@ ebiptablesCreateRuleInstanceIterate( virNWFilterRuleInstPtr res) { int rc = 0; - virNWFilterVarCombIterPtr vciter; + virNWFilterVarCombIterPtr vciter, tmp;
/* rule->vars holds all the variables names that this rule will access. * iterate over all combinations of the variables' values and instantiate * the filtering rule with each combination. */ - vciter = virNWFilterVarCombIterCreate(vars, - rule->varAccess, rule->nVarAccess); + tmp = vciter = virNWFilterVarCombIterCreate(vars, + rule->varAccess, rule->nVarAccess);
ACK with this line wrapped (too long). Martin
participants (2)
-
Daniel P. Berrange
-
Martin Kletzander