Re: [libmicrohttpd] memory issue in MHD_create_response_from_data

[Christian Grothoff]

In general, having two implementations for malloc/free in the same application 
is a terrible idea.  Compiling your C/C++ code with different compilers 
against different libc's is an equally terrible idea and will likely cause all 
kinds of problems, not just with MHD.  

Now, even in that case there is no reason to change the MHD API as you 
suggested; simply use the other API function to create a response:

struct MHD_Response *MHD_create_response_from_callback (uint64_t size,
                                                        size_t block_size,
                                                        
MHD_ContentReaderCallback
                                                        crc, void *crc_cls,
                                                        
MHD_ContentReaderFreeCallback
                                                        crfc);

there you already have your "free" callback where you can do as you please. 
MHD_create_response_from_data is supposed to be the simple method for the 
simple case of responses that are small enough to fit into memory and that are 
either on the stack, in an immutable global or malloc'ed (where the malloc 
must match the malloc that MHD would be using).  If that's not the case, the 
other way to create a response object should do just fine.

Best,

Christian

On Saturday 07 February 2009 12:36:02 pm Alexander Antimonov wrote:
> Christian Grothoff wrote:
> > On Friday 06 February 2009 09:45:59 am you wrote:
> >> When I used dynamic memory allocated by new operator and gave to
> >> MHD_create_response_from_data(size, newed_point, MHD_YES, MHD_NO), I got
> >> double freeing memory crash while I did not delete the dynamic memory in
> >> my own code.
> >
> > First of all, you should not use "new" for this, use "malloc".  This is a
> > C library, not C++.
> >
> > Christian
>
> I've just realized that interface of the MHD_create_response_from_data
> is not so good. Even if "malloc" is used instead of the "new", there is
> still an issue.
>
> Let me explain.
> Say, we have an executable and a shared object.
> A general rule for such kind of interaction is if one of the sides
> creates an object and passes its ownership to the other side, the first
> side should provide corresponding freeing function for the object. When
> the side which has taken the ownership decides to release the object, it
> calls the freeing function provided by the first side.
> The same is correct in the opposite direction.
>
> In the MHD there are:
> -   "struct MHD_Response* MHD_create_response_from_data()"
>      and corresponding function "void MHD_destroy_response()"
>
> -   "struct MHD_PostProcessor* MHD_create_post_processor()"
>      and corresponding function "int MHD_destroy_post_processor()"
>
> If one side allocates memory with "malloc" the other side should use the
> "free" function from the first side to free that memory.
>
> Well, maybe current MHD_create_response_from_data will work if both
> sides are compiled by the same compiler (and use the same "C" runtime),
> it almost definitely will not if compilers is different (problem point
> is linking phase).
>
> Why it is so you can read in details here:
> http://lists.trolltech.com/qt-interest/2003-01/thread00220-0.html
> the right answers start from message 12.
>
> In the light of the above, I think it's better to change interface of
> the MHD_create_response_from_data to something like this:
>
>    typedef void(*) MHD_DataFreeCallback (void *cls);
>
>    struct MHD_Response* MHD_create_response_from_data(
>                         size_t size,
>                         void * data,
>                         MHD_DataFreeCallback crdfc,
>                         int must_copy);
>
> crdfc == NULL is equivalent to must_free == MHD_NO,
> crdfc != NULL is equivalent to must_free == MHD_YES, and crdfc should be
> used by MHD to free the data. In this case it doesn't matter what was
> used to allocate memory: "new" or "malloc".
>
> _______________________________________________
> libmicrohttpd mailing list
> address@hidden
> http://lists.cs.du.edu/mailman/listinfo/libmicrohttpd