Re: [PATCH nft 2/4] gmputil: add nft_gmp_free() to free strings from mpz_get_str()

[Thomas Haller <thaller@redhat.com>]

On Wed, 2023-09-20 at 16:05 +0200, Phil Sutter wrote:
> On Wed, Sep 20, 2023 at 03:13:39PM +0200, Thomas Haller wrote:
> > 
> > +       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?

Hi Phil,


no, it's not expected to EVER change. Users must not change
mp_set_memory_functions() after any GMP objects were allocated,
otherwise there would be a mixup of allocators and crashes ahead.

However, I didn't cache the value, because I don't want to use global
data without atomic compare-exchange (or thread-local). Doing it
without regard of thread-safety so would be a code smell (even if
probably not an issue in practice). And getting it with atomic/thread-
local would be cumbersome. It's hard to ensure a code base has no
threading issues, when having lots of places that "most likely are
99.99% fine (but not 100%)". Hence, I want to avoid the global.

I think the call to mp_get_memory_functions() should be cheap.

Note that libnftables no longer calls mp_set_memory_functions() ([1]).
So for the patch to have practical effects, you would need to have
another part of the process call mp_set_memory_functions() and set a
free function incompatible with libc's free(). So the scenario is very
unlikely already. It's more about being clear about using the correct
free for an allocation (even if in practice it all ends up being the
same).

[1] https://git.netfilter.org/nftables/commit/?id=96ee78ec4a0707114d2f8ef7590d08cfd25080ea


Thomas