Re: ber_memfree debugging

["Kurt D. Zeilenga" <Kurt@OpenLDAP.org>]

At 09:39 PM 6/6/99 +0200, Bert Vermeulen wrote:
>ber_memfree() in liblber has an assertion on the memory to be freed.
>Should this be there?

That's debatable. 

I am currently using assert() to detect portability bugs in our
memory allocation codes.  This assert actually demonstrates
such a bug.

>It's bombing out on me because
>ldap_str2attributetype() in libldap frees pointers regardless if they've
>been used or not.

Before the recent memory allocation changes, this code called
free(NULL).  This is a bug.  OpenLDAP should not assume
STDC semantics.  I've added asserts specifically to detect
such bugs.

Now, one could say that LDAP_FREE() should provide should
provide STDC semantics even where free() doesn't.  In the
long run, I concur.  In fact, in the long run, I rather
eliminate most of the caller NULL checks.  This allows
LDAP_FREE to be used to implement other sorts of deallocation
bugs (such as duplicate frees).

Here an off the wall suggestion.

Round 3: (we gone a couple already on this)
	ber_memfree: don't assert(p != NULL)
	LDAP_FREE: assert(p != NULL)
	LDAP_INT_FREE: don't assert(p != NULL)

	Where LDAP_FREE(p) asserts.  Review code for usage
	and change to either:
		if(p) LDAP_FREE(p);
	or
		LDAP_INT_FREE(p);

Round 4:
	Phase out LDAP_FREE in favor of LDAP_INT_FREE.
	Possibly change LDAP_INT_FREE to do double free
	and/or use after free detection.

>It looks to me like the assertion should happen within the #ifdef
>LDAP_MEMORY_DEBUG block...

The LDAP_MEMORY_DEBUG enables detect wrong heap deallocation
detection which is something completely different.  It's not
test enough to be included as part of general debugging
(LDAP_DEBUG) and general debugging isn't ready for it.