Re: [libnftnl PATCH] utils: fix buffer reallocation of nft_fprinft()

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Arturo,

On Fri, May 09, 2014 at 06:45:47PM +0200, Arturo Borrero Gonzalez wrote:
> When _snprintf() reports it would print n characters, that n doesn't include
> the trailing \0 that snprintf adds.
> 
> Thus, we need to [re]allocate n+1 characters.
> 
> While at it, change the reallocation trigger. If the length of the buffer we
> used is equals to the expanded string length, the output has been truncated.
> In other words, if ret == bufsiz, then the trailing \0 is missing.
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  src/utils.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/utils.c b/src/utils.c
> index 18917f5..b8094aa 100644
> --- a/src/utils.c
> +++ b/src/utils.c
> @@ -195,12 +195,13 @@ int nft_fprintf(FILE *fp, void *obj, uint32_t type, uint32_t flags,
>  	int ret;
>  
>  	ret = snprintf_cb(buf, bufsiz, obj, type, flags);

I think we should also check here if snprintf returns -1 now that you
have fixed the SNPRINTF_ macro.

> -	if (ret > NFT_SNPRINTF_BUFSIZ) {
> -		buf = calloc(1, ret);
> +	if (ret >= NFT_SNPRINTF_BUFSIZ) {
> +		bufsiz = ret + 1;
> +
> +		buf = calloc(1, bufsiz);

You can use malloc instead. Just make sure that the string is always
nul-terminated before printing, something like:

                bufsiz = ret + 1;
                buf = malloc(1, bufsiz);
                if (buf == NULL)
                        return -1;

                ret = snprintf(...
                if (ret < 0)
                        ...
        }

        buf[ret] = '\0';
        ... = fprintf(...

Thanks.