>>1.) Added --enable-memcallback flag. This has no effect when --enable-debug
>>is used.
>
> I think this code should be unconditionally used, but this approach certainly
> makes it easy to enable/disable this during the initial test period.

Are you saying you want this to be enabled by default, but have an
option to turn it off? Or there is no need to have ability to disable
this feature?

>>2.) Added curl_set_mem_callbacks function in curl.h when CURL_MEMCALLBACK
>>CPPFLAG is set (will be set, if compiled with --enable-memcallback) This
>>function can be called with user defined memory management routines.
>
> The API needs to be present at all times. We don't want conditionally present
> functions in libcurl.

Should be OK. Whether the function actually does something or is a
no-op, depends on whether we have an option to disable this funtionality
or not.

>>3.) Replaced malloc/free/realloc/strdup with
>>curl_malloc/curl_free/curl_realloc/curl_strdup respectively in all .c files
>>in lib and src folders.
>
> I think I would rather see that you keep the names malloc/free/realloc/strdup
> etc in all the code. Not only are the commonly used and understood, but in the
> future when people provide patches it is much simplier and easier if we
> continue to use the "standard" function names. Also, not having them named
> like that increases the risk that such ones "sneak in" in the future.
>
> We could instead just #define those names to use the proper global function
> pointers instead, very similar to how the memory debug system already works.
>
> It would also make your patch MUCH simpler.

It will definitely make the patch much easier, if the standard function
names can be kept. Yes there is the issue of someone sneaking in code
with malloc/free etc. I figure there is an interesting way to do this.
But then we will still have to rename some functions (like the free in
url.c where the memory is allocated by libIDN).

>>4.) url.c has 2 'free' calls that are not changed, since the memory is
>>allocated externally in libidn.
>
> Good, we must make sure that we maintain this ability.

As I mentioned above, we might have to rename these. We don't want the
callback functions to be used for these.

>>5.) Replaced all calloc's with curl_malloc and memset.
>
> I don't like that. We just recently introduced calloc() after years of not
> having it, and I think that it is useful. Again, for conveniance to
> developers. I'd rather let the app that provides the callbacks do that
> malloc()-then-memset() logic.

I guess that mean one more callback function!

>>9.) I am not sure if this is problematic: I removed "curl_free" function
>>from curl.h. I did not understand why it is there in the first place. It is
>>just a wrapper around 'free'. It is not consistently used in the curl code.
>>I am not sure, what functionality it is supposed to provide to the libCurl
>>library users. Let me know if this is an issue, I will add it back and
>>rename my curl_free macro to something else....
>
> It is problematic because curl_free() is useful and fills a purpose. It
> doesn't have to be used within libcurl so there's no inconsistancy there to
> worry about.
>
> The whole point of curl_free() is to allow applications to free memory
> allocated by libcurl. By providing curl_free() the app makes sure that the
> memory is freed with/by the same memory subsystem that allocated the memory.
> There are two notable use cases for this:
>
> 1. In Windows, you can link the lib and the app with two different memory
> subsystems that makes it impossible for the app to free memory that the
> lib allocated, unless this function is provided.
>
> 2. When we build libcurl with memory debugging enabled, we add data around
> each malloced data chunk that needs to be taken care of when the data is
> freed. By providing curl_free(), any ordinary libcurl-using app can
> link/use a libcurl that is built with memory-debugging without having to
> care the least.

Got it.

Let me work on the new patch. I will mail it to the list as soon as I
have it.

Regards
-Seshubabu Pasam
Received on 2004-05-07