On Fri, Aug 12, 2016 at 02:44:58AM +0200, Phil Sutter wrote:
> On Fri, Aug 12, 2016 at 01:42:02AM +0200, Pablo Neira Ayuso wrote:
> > On Fri, Aug 12, 2016 at 01:33:34AM +0200, Phil Sutter wrote:
> > > From: Phil Sutter <psutter@xxxxxxxxxx>
> > >
> > > This is an ugly aspect of the SNPRINTF_BUFFER_SIZE() macro: it contains
> > > a return statement and if that triggers, the function returns without
> > > freeing the iterator object. Therefore duplicate the 'ret < 0' check
> > > before calling it, freeing the iterator knowing that we will bail out
> > > immediately afterwards anyway.
> > >
> > > Cc: Arturo Borrero <arturo.borrero.glez@xxxxxxxxx>
> > > Signed-off-by: Phil Sutter <phil@xxxxxx>
> > > ---
> > > src/ruleset.c | 16 ++++++++++++++++
> > > 1 file changed, 16 insertions(+)
> > >
> > > diff --git a/src/ruleset.c b/src/ruleset.c
> > > index 666bcc7a246b6..93cf95ab61e15 100644
> > > --- a/src/ruleset.c
> > > +++ b/src/ruleset.c
> > > @@ -888,12 +888,16 @@ nftnl_ruleset_snprintf_table(char *buf, size_t size,
> > > t = nftnl_table_list_iter_next(ti);
> > > while (t != NULL) {
> > > ret = nftnl_table_snprintf(buf+offset, len, t, type, flags);
> > > + if (ret < 0)
> > > + nftnl_table_list_iter_destroy(ti);
> > > SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> >
> > Better get rid of the obscure if (ret < 0) hidden in
> > SNPRINT_BUFFER_SIZE.
> >
> > Or simply set:
> >
> > if (ret < 0)
> > ret = 0;
> >
> > in SNPRINT_BUFFER_SIZE.
>
> Hmm. This means we will lose error propagation. Given how widely this
> macro is being used (grep says 200 calls), this needs a good second
> thought.
The usual suggested idiom to deal with this looks like:
snprintf(cp, blen, "AF=%d ", sau->soa.sa_family)
blen -= strlen(cp);
cp += strlen(cp);
See: https://goo.gl/AUGE5m
No need to second thought, we can just ignore the -1 case.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
