Sorry, the patch was inverted, here is a clean patch
Le 19-07-21 à 22 h 30, Nicolas Mora a écrit :
> Hello again,
>
> In fact, there already was the skeleton in MHD to provide what I mean. I
> attached a patch fil for the function MHD_set_response_options that
> allows to override free() for the response buffer.
> The change is quite simple in fact...
>
> I didn't add tests to validate this patch because I wouldn't know where
> to put them.
> But with this change, I don't need to use MHD_RESPMEM_MUST_COPY anymore!
>
> What do you think?
>
> /Nicolas
>
> Le 19-07-21 à 16 h 10, Nicolas Mora a écrit :
>> Hello,
>>
>> Thanks for the feedback. I totally agree with you on the performance
>> side. I myself don't use different malloc()/realloc()/free() than the
>> libc one.
>>
>> The only reason I make this request is because of a bug a user of my
>> framework had because of my use of MHD_RESPMEM_MUST_FREE by default
>> which caused problems if the response is allocated with a malloc()
>> function incompatible with the libc one.
>>
>> The workaround I found was to use MHD_RESPMEM_MUST_COPY instead, then
>> free the bdy response after calling MHD_create_response_from_buffer.
>> This works fine that some memory resource is wasted (not for long but
>> still).
>>
>> If not for the specific malloc/free function, would it be possible to
>> specify the free() function to deallocate the response body when using
>> MHD_RESPMEM_MUST_FREE? Like an option you could pass to MHD_Connection *
>> inside the MHD_AccessHandlerCallback function?
>>
>> Le 19-07-21 à 15 h 44, Christian Grothoff a écrit :
>>> Hi Nicolas,
>>>
>>> Thanks for the proposal, but I don't think this kind of patch belongs
>>> into MHD. malloc() performance is not critical for MHD at all, as MHD
>>> hardly uses malloc(): We mostly use our own custom memory pool, usually
>>> on top of mmap(), to avoid fragmentation issues and to limit memory
>>> consumption per TCP connection.
>>>
>>> So I doubt you'd get _any_ performance delta by using Hoard. If I am
>>> wrong and you do have MHD-specific performance measurements that show
>>> that this is not premature optimization, please share them!
>>>
>>> Please also consider that there are allocation functions like
>>> strdup()/strndup(), and mixing allocators (malloc going to Hoard,
>>> strdup() to libc) is likely to end in disaster on free(). So in my view
>>> the only _good_ place to add the functions you propose (or Hoard itself)
>>> would be (GNU) libc.
>>>
>>> Happy hacking!
>>>
>>> Christian
>>>
>>> On 7/21/19 9:28 PM, Nicolas Mora wrote:
>>>> I happened to see that MHD doesn't allow to use different
>>>> malloc/calloc/free functions than the one provided by libc.
>>>>
>>>> This would be useful if the underlying app using MHD uses different
>>>> allocating functions like Hoard: http://www.hoard.org/
>>>> More specifically, when using MHD_RESPMEM_MUST_FREE in a response
>>>> allocated with a different malloc() function, there would be problems.
>>>>
>>>> I can submit a patch for it.
>>>> Basically, I'd do it by adding functions like this:
>>>>
>>>> void MHD_set_alloc_funcs(MHD_malloc_t malloc_fn, MHD_calloc_t calloc_fn,
>>>> MHD_free_t free_fn);
>>>> void MHD_get_alloc_funcs(MHD_malloc_t * malloc_fn, MHD_calloc_t *
>>>> calloc_fn, MHD_free_t * free_fn);
>>>>
>>>> I didn't see any use of realloc() in the source code, so I wouldn't
>>>> allow to change it.
>>>>
>>>> Then, all internal call to malloc()/calloc()/free() would be replaced by
>>>> MHD_malloc()/MHD_calloc()/MHD_free()
>>>>
>>>> How about that? Any feedback?
>>>>
>>>> /Nicolas
>>>>
>>>
>>
diff -x Makefile -x .deps -x '*.rc' -ur libmicrohttpd-0.9.65/src/include/microhttpd.h libmicrohttpd/src/include/microhttpd.h
--- libmicrohttpd-0.9.65/src/include/microhttpd.h 2019-07-05 16:21:24.000000000 -0400
+++ libmicrohttpd/src/include/microhttpd.h 2019-07-21 22:03:05.388191487 -0400
@@ -2887,11 +2887,24 @@
/**
* End of the list of options.
*/
- MHD_RO_END = 0
+ MHD_RO_END = 0,
+
+ /**
+ * Set a specific free() function
+ * to free response buffer instead of libc void free(void * ptr)
+ */
+ MHD_RO_FREE_FUNCTION = 0
};
/**
+ * This typedef is defined to be able to pass a function pointer
+ * as a va_arg in MHD_set_response_options
+ */
+typedef void(*MHD_free_ptr)(void *);
+
+
+/**
* Set special flags and options for a response.
*
* @param response the response to modify
diff -x Makefile -x .deps -x '*.rc' -ur libmicrohttpd-0.9.65/src/microhttpd/response.c libmicrohttpd/src/microhttpd/response.c
--- libmicrohttpd-0.9.65/src/microhttpd/response.c 2019-06-23 15:20:42.000000000 -0400
+++ libmicrohttpd/src/microhttpd/response.c 2019-07-21 21:51:46.837784832 -0400
@@ -400,7 +400,6 @@
return response;
}
-
/**
* Set special flags and options for a response.
*
@@ -425,6 +424,15 @@
{
switch (ro)
{
+ case MHD_RO_FREE_FUNCTION:
+ va_start (ap, flags);
+ if (NULL != (response->crfc = va_arg (ap, MHD_free_ptr))) {
+ ret = MHD_YES;
+ } else {
+ ret = MHD_NO;
+ }
+ va_end (ap);
+ break;
default:
ret = MHD_NO;
break;
