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
