Re: ldap_msgfree() when ldap_search_s() failed

[Kazuaki Anami <kazuaki@soum.co.jp>]

From: Brian Candler <B.Candler@pobox.com>
Subject: Re: ldap_msgfree() when ldap_search_s() failed
Date: Wed, 1 Jun 2005 09:15:55 +0100

> > > After executing ldap_search_s() or ldap_result(), it seems to have to
> > > execute ldap_msgfree() regardless of the success or failure. Is it
> > > right?
> 
> Can you be more specific about "success or failure"?

I think only the case ldap_search_s() returned LDAP_SUCCESS is 
"success". The other cases, LDAP_NO_SUCH_OBJECT for example, are
"failure".

> If I understand rightly, ldap_result() sets a pointer to an LDAPMessage 
> structure, in the event that a message is available. If it does, you have to 
> free it. This will be the case even if your search returns zero entries, for 
> instance (which is a "successful" search).

Yes, the case no entry found in directory causes the problem.

I agree the case no entry found is a successful search, but
ldap_search_s() doesn't return LDAP_SUCCESS. 

> However, if ldap_result() itself fails (returning -1 for error or 0 for 
> timeout waiting for a result), then I don't think that anything is allocated, 
> so there's nothing to free.

I got it. ldap_result() has no problem.

> So, can you give an example of a code snippet which is not behaving as you 
> expect, explaining what actually happens, and what you think should happen 
> instead?

My code below causes memory leak when found no entry.

    int rv;
    LDAPMessage *msg;

    if ((rv = ldap_search_s(..., msg)) != LDAP_SUCCESS) {
    	puts(ldap_err2string(rv));
    	return rv;
    }

I added "ldap_msgfree(msg);" before "return rv;", and memory leak was
solved. 

Now, I have to know whether ldap_msgfree() is safe regardless of the
returned value, rv. If NULL or a valid value which can be freed with
ldap_msgfree() is surely set in msg, ldap_msgfree() is "safe".

If ldap_msgfree() is not safe, my code should be below. And I have to
know the cases I have to execute ldap_msgfree() besides the case rv is 
LDAP_NO_SUCH_OBJECT.

    if ((rv = ldap_search_s(..., msg)) != LDAP_SUCCESS) {
    	puts(ldap_err2string(rv));
	switch(rv) {
		case LDAP_NO_SUCH_OBJECT:
			ldap_msgfree(msg);
			break;
		default:
			/* Is msg NULL? */
	}
    	return rv;
    }

thanks.
____________________________________
 Kazuaki Anami (kazuaki@soum.co.jp)