Hi David, Owen, > -----Original Message----- > From: David Marchand <david.march...@redhat.com> > Sent: Tuesday, June 22, 2021 8:24 AM > To: Owen Hilyard <ohily...@iol.unh.edu>; Iremonger, Bernard > <bernard.iremon...@intel.com>; Yigit, Ferruh <ferruh.yi...@intel.com> > Cc: dev <dev@dpdk.org> > Subject: Re: [PATCH] lib/flow_classify: fix leaking rules on delete > > On Wed, Jun 16, 2021 at 9:57 PM <ohily...@iol.unh.edu> wrote: > > > > From: Owen Hilyard <ohily...@iol.unh.edu> > > > > Rules in a classify table were not freed if the table had a delete > > function. > > > > Fixes: be41ac2a3 ("flow_classify: introduce flow classify library") > Cc: sta...@dpdk.org > > > > > Signed-off-by: Owen Hilyard <ohily...@iol.unh.edu> > > --- > > lib/flow_classify/rte_flow_classify.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/flow_classify/rte_flow_classify.c > > b/lib/flow_classify/rte_flow_classify.c > > index f125267e8..06aed3b70 100644 > > --- a/lib/flow_classify/rte_flow_classify.c > > +++ b/lib/flow_classify/rte_flow_classify.c > > @@ -579,7 +579,7 @@ rte_flow_classify_table_entry_delete(struct > rte_flow_classifier *cls, > > &rule->u.key.key_del, > > &rule->key_found, > > &rule->entry); > > - > > + free(rule); > > return ret; > > } > > } > > I find it strange to free the rule regardless of the result of the > f_delete() op.
I agree the result of the f_delete() op should be checked before freeing the rule. > The same is done out of the loop which means this function returns -EINVAL > and frees the rule in this case too. The free() outside the loop at line 587 does not make sense to me now and should be removed. > > Bernard, Ferruh, can you review please? > > Thanks! > > > -- > David Marchand Regards, Bernard.