On Mon, Oct 28, 2024 at 11:59:23PM +0100, Pablo Neira Ayuso wrote:
> On Wed, Oct 23, 2024 at 10:21:04PM +0200, Phil Sutter wrote:
> [...]
> > @@ -325,12 +295,11 @@ void nftnl_flowtable_nlmsg_build_payload(struct nlmsghdr *nlh,
> >
> > if (c->flags & (1 << NFTNL_FLOWTABLE_DEVICES)) {
> > struct nlattr *nest_dev;
> > + const char *dev;
> >
> > nest_dev = mnl_attr_nest_start(nlh, NFTA_FLOWTABLE_HOOK_DEVS);
> > - for (i = 0; i < c->dev_array_len; i++) {
> > - mnl_attr_put_strz(nlh, NFTA_DEVICE_NAME,
> > - c->dev_array[i]);
> > - }
> > + nftnl_str_array_foreach(dev, &c->dev_array, i)
>
> Where is this nftnl_str_array_foreach defined? I don't find it in this
> patch.
Argh, not just did I forget to 'git add' the new files, I also missed
that it's about two files, not just one. I'll send a v3 which also
includes str_array.h. Sorry for the mess.
> [...]
> > +void nftnl_str_array_clear(struct nftnl_str_array *sa)
> > +{
> > + while (sa->len > 0)
> > + free(sa->array[--sa->len]);
> > + free(sa->array);
> > + sa->array = NULL;
>
> This is new, I'm fine with this, but it is only defensive, right?
> This stale reference would not be reached because attribute flag is
> cleared.
Oh, you're right. A first approach used realloc_array() in
nftnl_str_array_set(), but I abandoned it and this is a left-over.
I'll drop it from v3.
> > +}
> [...]
> > diff --git a/src/utils.c b/src/utils.c
> > index 2f1ffd6227583..157b15f7afe8d 100644
> > --- a/src/utils.c
> > +++ b/src/utils.c
> > @@ -19,6 +19,7 @@
> >
> > #include <libnftnl/common.h>
> >
> > +#include <libmnl/libmnl.h>
> > #include <linux/netfilter.h>
> > #include <linux/netfilter/nf_tables.h>
>
> Remove this chunk? It looks unrelated.
ACK, sorry. Also a left-over.
Thanks, Phil
