On Wed, Sep 20, 2023 at 03:13:39PM +0200, Thomas Haller wrote:
> mpz_get_str() (with NULL as first argument) will allocate a buffer using
> the allocator functions (mp_set_memory_functions()). We should free
> those buffers with the corresponding free function.
>
> Add nft_gmp_free() for that and use it.
>
> The name nft_gmp_free() is chosen because "mini-gmp.c" already has an
> internal define called gmp_free(). There wouldn't be a direct conflict,
> but using the same name is confusing. And maybe our own defines should
> have a clear nft prefix.
>
> Signed-off-by: Thomas Haller <thaller@xxxxxxxxxx>
> ---
> include/gmputil.h | 2 ++
> src/evaluate.c | 6 +++---
> src/gmputil.c | 21 ++++++++++++++++++++-
> 3 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/include/gmputil.h b/include/gmputil.h
> index c524aced16ac..d1f4dcd2f1c3 100644
> --- a/include/gmputil.h
> +++ b/include/gmputil.h
> @@ -77,4 +77,6 @@ extern void __mpz_switch_byteorder(mpz_t rop, unsigned int len);
> __mpz_switch_byteorder(rop, len); \
> }
>
> +void nft_gmp_free(void *ptr);
> +
> #endif /* NFTABLES_GMPUTIL_H */
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 03586922848a..e5c7e03a927f 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -401,7 +401,7 @@ static int expr_evaluate_integer(struct eval_ctx *ctx, struct expr **exprp)
> expr_error(ctx->msgs, expr,
> "Value %s exceeds valid range 0-%u",
> valstr, ctx->ectx.maxval);
> - free(valstr);
> + nft_gmp_free(valstr);
> return -1;
> }
>
> @@ -417,8 +417,8 @@ static int expr_evaluate_integer(struct eval_ctx *ctx, struct expr **exprp)
> expr_error(ctx->msgs, expr,
> "Value %s exceeds valid range 0-%s",
> valstr, rangestr);
> - free(valstr);
> - free(rangestr);
> + nft_gmp_free(valstr);
> + nft_gmp_free(rangestr);
> mpz_clear(mask);
> return -1;
> }
> diff --git a/src/gmputil.c b/src/gmputil.c
> index bf472c65de48..550c141294a3 100644
> --- a/src/gmputil.c
> +++ b/src/gmputil.c
> @@ -185,7 +185,7 @@ int mpz_vfprintf(FILE *fp, const char *f, va_list args)
>
> str = mpz_get_str(NULL, base, *value);
> ok = str && fwrite(str, 1, len, fp) == len;
> - free(str);
> + nft_gmp_free(str);
>
> if (!ok)
> return -1;
> @@ -197,3 +197,22 @@ int mpz_vfprintf(FILE *fp, const char *f, va_list args)
> return n;
> }
> #endif
> +
> +void nft_gmp_free(void *ptr)
> +{
> + void (*free_fcn)(void *, size_t);
> +
> + /* When we get allocated memory from gmp, it was allocated via the
> + * allocator() from mp_set_memory_functions(). We should pair the free
> + * with the corresponding free function, which we get via
> + * mp_get_memory_functions().
> + *
> + * It's not clear what the correct blk_size is. The default allocator
> + * function of gmp just wraps free() and ignores the extra argument.
> + * Assume 0 is fine.
> + */
> +
> + mp_get_memory_functions(NULL, NULL, &free_fcn);
Do we have to expect the returned pointer to change at run-time? Because
if not, couldn't one make free_fcn static and call
mp_get_memory_functions() only if it's NULL?
Cheers, Phil
