Re: [libmicrohttpd] must_free & must_copy for MHD_create_response_from_data

[Christian Grothoff]

On Wednesday, December 29, 2010 12:50:15 pm Basile Starynkevitch wrote:
> Hello All,
> 
> First, a small wish: for readability purposes, add at least
> enum { MHD_DONT_FREE=0,
>        MHD_MUST_FREE};
> enum { MHD_DONT_COPY=0,
>        MHD_MUST_COPY};
> to microhttpd.h (or the equivalent #define-s).
> 
> I find much more readable a call like:
>   resp = MHD_create_response_from_data (mysize,
>                                         mydata,
>                                         MHD_MUST_FREE, MHD_MUST_COPY);
> than just
>   resp = MHD_create_response_from_data (mysize,
>                                         mydata,
>                                         MHD_YES, MHD_YES);
> 
> Second, a question:
> Does haveing the must_free argument to false and must_copy argument to true
> makes sense? I suggest at least adding a comment explaining what happens
> in that case.

'must_free' is ignored if 'must_copy' is set to YES, so setting it to anything 
doesn't really matter at that point.
 
> Perhaps a bettter signature would be
> 
> /* mode for handling data for responses */
> enum mhd_response_memory_mode_en {
>   /* the data won't be freed or copy by libmicrohttpd, e.g. because it is a
> static buffer */ MHD_RESPMEM_NONE,
> 
>   /* the data will be freed by libmicrohttpd when done. */
>   MHD_RESPMEM_MUST_FREE,
> 
>   /* the data is copied to a private buffer and will be then freed by
> libmicrohttpd when done. */ MHD_RESPMEM_MUST_COPY
> };
>
> And add a call
> 
> /**
>  * Create a response object.  The response object can be extended with
>  * header information and then be used any number of times.
>  *
>  * @param size size of the data portion of the response
>  * @param data the data itself
>  * @param mode the mode to handle the data
>  * @return NULL on error (i.e. invalid arguments, out of memory)
>  */
> struct MHD_Response* MHD_create_response_from_data_modal (size_t size,
>                                                           void* data,
>                                                           enum
> mhd_response_memory_mode_en mode);

I'll do something along those lines (currently testing, check SVN 
tomorrow...).

Happy hacking,

Christian