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
