Re: [libnf_ct PATCH v2 1/9] Handle negative snprintf return values properly

["Daniel Gröber" <dxld@darkboxed.org>]

On Wed, Jul 01, 2020 at 01:09:51PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Jun 24, 2020 at 03:29:57PM +0200, Daniel Gröber wrote:
> > Currently the BUFFER_SIZE macro doesn't take negative 'ret' values into
> > account. A negative return should just be passed through to the caller,
> > snprintf will already have set 'errno' properly.
> 
> Series applied, thanks.

Great, thanks!

> > diff --git a/include/internal/internal.h b/include/internal/internal.h
> > index bb44e12..b1fc670 100644
> > --- a/include/internal/internal.h
> > +++ b/include/internal/internal.h
> > @@ -41,6 +41,8 @@
> >  #endif
> >  
> >  #define BUFFER_SIZE(ret, size, len, offset)		\
> > +	if (ret < 0)					\
> > +		return -1;				\
> 
> Side note: I don't like this hidden branch under a macro. But
> snprintf() == -1 is unlikely to happen and I don't have a better idea
> to deal with this case ATM.

Since there is already another branch in this macro I take it the issue is
the part where we return now, right?

I can see how this is kind of nasty being completely implicit and all, how
about using a label and passing the name to the macro? Something like:

    ret = snprintf(...)
    BUFFER_SIZE_ERR(ret, size, len, offset, err);
    [...]
    err:
        return -1;

--Daniel