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
