See also https://bugs.php.net/bug.php?id=74808.

https://bugs.php.net/bug.php?id=74808

bug isn't public

we already have SetMethod APIs for hooks like errors. so just expand that ?

• gdSetMemoryFreeMethod
• gdSetMemoryMallocMethod
• gdSetMemoryReallocMethod
• etc...

What about thread safety? I've found that using that using a static struct doesn't lead to a major architecture change or a rewrite of the library.

what about thread safety are you worried about ?

I'm worried about changing the allocation methods during application runtime, but I think is much clever to mention that is unwise to change the methods that often.

bug isn't public

The report doesn't really add new info. :)

I'm worried about changing the allocation methods during application runtime, […]

I think that would be a really bad idea anyway, and should not be allowed (at least by documentation).

gdSetMemoryFreeMethod() and friends is fine for me, but I also can imagine having a single function which accepts all the callbacks, similar to u_setMemoryFunctions.

I was using this struct for my prototypes.

typedef struct gdallochelpers
{
	void *(*callocf)(size_t nmemb, size_t size);
	void *(*mallocf)(size_t size);
	void *(*reallocf)(void *ptr, size_t size);
	void (*freef)(void *ptr);
} gdAllocHelpers;

If a u_setMemoryFunctions equivalent is done we could pass the functions as parameters or packed in the struct.

An API could be described like this:

gdSetMemoryCallocMethod(void *(*callocf)(size_t nmemb, size_t size));
gdSetMemoryMallocMethod(void *(*mallocf)(size_t size));
gdSetMemoryReallocMethod(void *(*reallocf)(void *ptr, size_t size));
gdSetMemoryFreeMethod(void (*freef)(void *ptr));
gdSetMemoryAllocationMethods(gdAllocHelper helper);

Error handling may be optional if desired (i.e. returning something else instead of void). Also in the case of passing NULL function pointers to gdSetMemoryAllocationMethods I'd suggest to just fallback to the corresponding default method.

i agree switching memory allocators while other threads are actively using gd seems like a terrible idea and not something we can reasonably protect against. imagine:

• [10 min ago] Thread1 creates an image in memory
• [9 min ago] Thread2 changes allocator
• [8 min ago] Thread1 frees the image

which allocator should T1 use ? we'd have to pin the allocator information to the image state itself, but even then, you'd be left with T1 accessing the old allocator long in the future which might not be expected. the atomicity of updates for the hooks/methods wouldn't change this problem.

we should just document it as "pick an allocator before using gd and leave it at that".

having a struct wouldn't help with atomic updates either. doing a memcpy of the struct is not guaranteed ... we'd have to maintain 2 structs and add memory barriers everywhere which sounds like overkill.

so the only thing using a struct gains us is having a single API for users to call vs ~5 APIs. the reason i didn't suggest exporting a gdAllocHelpers struct is to not worry about ABI compatibility of it if we want to add or remove members in the future. methods make that a bit easier, and i'm not sure that one func w/a struct argument is easier for users than a bunch of method calls.

  gdAllocHelpers foo = {
    .malloc = mymalloc,
    .calloc = mycalloc,
    .free = myfree,
  };
  gdSetMemoryAllocationMethods(&foo);
...
  gdSetMemoryMallocMethod(mymalloc);
  gdSetMemoryCallocMethod(mycalloc);
  gdSetMemoryFreeMethod(myfree);

The purpose of the struct was more of a holder for the methods, but any other solution could be used instead.

The gdSetMemoryAllocationMethods should be used to set the methods in one fell swoop. This description of the method may be the midpoint of having the functionality and not exposing gdAllocHelpers:

gdSetMemoryAllocationMethods(void *(*callocf)(size_t nmemb, size_t size), void *(*mallocf)(size_t size), void *(*reallocf)(void *ptr, size_t size), void (*freef)(void *ptr));

As of thread safety, all the described methods must be documented to mention that they are not thread-safe and that they should not be called during operations because we cannot guarantee both thread safety (in an easy way) and that there wouldn't be any issues by switching allocators oftentimes.

wrt ABI compatibility, a func that takes a struct of fields vs a func that takes all the fields directly is the same. if we want to add/remove/update any helpers, then we need to create a new func.

actually, a single func-with-many-args is kind of worse than if it just took a struct. at least with a struct we could make the first field a "version" and figure out the layout from there. a func we'd have to create a new symbol, or go the va_args route, and that'd be even more confusing for people.

if people really really want a single method that takes a (versioned) struct, i won't fight over it more. but a func that takes multiple arguments is out of the question.

@vapier My only worry is about three functions and users start to use incompatible *alloc and free by mistake, setting them all at once can avoid this, hopefully :)

@remicollet that is, afair, the last thing that could prevent to use only external GD with PHP

thoughts?

Yes, if this is available, PHP could drop bundled libgd. :)

@remicollet that is, afair, the last thing that could prevent to use only external GD with PHP

IMHO this have to be in small steps

• implement this feature
• sync ext/gd/libgd with upstream libgd so everyone use the same code (8.2 ?)
• later drop support for bundled library (8.3 ? 8.4 ?)

BTW, I have some bad experience with changing memory allocator (can't remember in which ext), which may raise strange issues if the same library is pulled by various projects in the same process, mixing different allocator. But I'm not aware of anything else using libgd (in the PHP stack)