Well, actually the man page is right but it's kinda tricky reading :)
Just depends on how you read it. It should be something like:
If the freeit parameter is non zero it indicates
that the res parameter should be freed by ldap_result2error
by a call to ldap_msgfree(3) after the error code has been extracted.
Anyway, the memory leak is indeed probably somewhere else. I might
try writing a test program in the next week or something to see if it
is rlm_ldap which is leaking or the whole OpenLDAP libs in general.
/Peter
-----Original Message-----
From: Juan Marchionatto [mailto:[EMAIL PROTECTED]]
Sent: Tuesday, September 04, 2001 4:27 PM
To: [EMAIL PROTECTED]
Subject: Re: (Freeradius 0.1 vs. Freeradius 0.2) + LDAP
Peter,
as you transcripted from the result2error ldap call
int ldap_result2error(LDAP *ld, LDAPMessage *res, int freeit);
* If the freeit parameter is non zero it indicates
* that the res parameter should be freed by a call
* to ldap_msgfree(3) after the error code has been extracted.
This means the 1 will NOT free the memory. Isn't it?
So this must be a leak source after all, as far as I can see.
And regarding the ldap_connect routine being called only at startup time,
this is not the only opportunity it is called, if I followed the code well
(which may be not, because I only did a visual check, as I don't have the
server running yet) .
Regards,
---
Juan
----- Original Message -----
From: "Peter Foreman" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
Sent: Monday, September 03, 2001 4:03 AM
Subject: RE: (Freeradius 0.1 vs. Freeradius 0.2) + LDAP
Not exactly. From the ldap_result man page:
Upon success, the type of the result received is returned
and the result parameter will contain the result of the
operation. This result should be passed to the LDAP pars
ing routines, ldap_first_entry(3) and friends, for inter
pretation.
ERRORS
ldap_result() returns -1 if something bad happens, and
zero if the timeout specified was exceeded.
That means that the first ldap_msgfree which you introduced is wrong, since
rc < 1 and that means that there was no success (and no res allocated
either).
For the second one, again from the manpage:
The ldap_result2error() routine takes res, a result as
produced by ldap_result(3) or ldap_search_s(3), and
returns the corresponding error code. Possible error
codes are listed below. If the freeit parameter is non
zero it indicates that the res parameter should be freed
by a call to ldap_msgfree(3) after the error code has been
extracted. The ld_errno field in ld is set and returned.
And if you look at the code:
ldap_errno = ldap_result2error(ld, res, 1);
The '1' tells ldap_result2error to free the res parameter. Also the second
ldap_msgfree you introduced should not be there!
/Peter
-----Original Message-----
From: John Morrissey [mailto:[EMAIL PROTECTED]]
Sent: Saturday, September 01, 2001 5:46 PM
To: [EMAIL PROTECTED]
Subject: Re: (Freeradius 0.1 vs. Freeradius 0.2) + LDAP
On Fri, Aug 31, 2001 at 10:34:15AM -0400, Juan Marchionatto wrote:
% Alan, Peter,
% am I missing something (again) or the ldap_connect routine should
% ldap_msgfree(res) before returning?
%
% This may be specially important when returning OK, because there is not
even
% an ldap_unbind (which might eventually free the memory) in that case.
You're right; the result passed to ldap_result() needs to be freed by the
caller. The diff below frees them properly.
I also read the source to rlm_ldap (albeit not as thoroughly as I'd like)
and couldn't find any more situations like this. Somehow I doubt that this
is the only source of the LDAP leaks I've seen reported. ldap_connect() only
gets called when the module is first initialized and when the LDAP server
goes away, so unless your LDAP server is bouncing like a rubber ball, I
don't see how this could account for the leaks I recall hearing about.
john
Index: rlm_ldap.c
===================================================================
RCS file: /source/radiusd/src/modules/rlm_ldap/rlm_ldap.c,v
retrieving revision 1.50
diff -u -r1.50 rlm_ldap.c
--- rlm_ldap.c 2001/08/28 20:01:48 1.50
+++ rlm_ldap.c 2001/09/01 15:28:41
@@ -640,10 +641,12 @@
ldap_get_option(ld, LDAP_OPT_ERROR_NUMBER, &ldap_errno);
radlog(L_ERR, "rlm_ldap: %s bind failed: %s", dn, (rc == 0)
? "timeout" : ldap_err2string(ldap_errno));
*result = RLM_MODULE_FAIL;
+ ldap_msgfree(res);
ldap_unbind_s(ld);
return (NULL);
}
ldap_errno = ldap_result2error(ld, res, 1);
+ ldap_msgfree(res);
switch (ldap_errno) {
case LDAP_SUCCESS:
*result = RLM_MODULE_OK;
--
John Morrissey _o /\ ---- __o
[EMAIL PROTECTED] _-< \_ / \ ---- < \,
www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__
-
List info/subscribe/unsubscribe? See
http://www.freeradius.org/list/users.html
-
List info/subscribe/unsubscribe? See
http://www.freeradius.org/list/users.html
-
List info/subscribe/unsubscribe? See
http://www.freeradius.org/list/users.html
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
