Re: [libmicrohttpd] feature request: change malloc/calloc/free functions

[Evgeny Grin]

Don't we have now duplication with
'MHD_create_response_from_buffer_with_free_callback()'?

Shouldn't we remove one of implementations?

-- 
Wishes,
Evgeny

22.07.2019 12:51, Christian Grothoff пишет:
> Thanks, that looks good --- modulo re-using the value 0 in the
> enumeration ;-), and lacking an update to the handbook/ChangeLog. I've
> made those fixes and pushed the result to Git:
> 
> 3d1b941137f9d8379e6e67d5abd57be5ae6ebe1a
> 
> Happy hacking!
> 
> Christian
> 
> On 7/22/19 4:33 AM, Nicolas Mora wrote:
>> 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
>>>>>>
>>>>>
>>>>
>

signature.asc
Description: OpenPGP digital signature