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.

Thanks,
---
Juan


----- Original Message -----
From: "Peter Foreman" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
Sent: Thursday, August 30, 2001 3:41 AM
Subject: RE: (Freeradius 0.1 vs. Freeradius 0.2) + LDAP


> This has been fixed by me some months ago, this bug was in rlm_ldap before
> but has been fixed a long time ago, I've been going thru the source
numerous
> of times and I can't seem to find the leak...
>
> -Peter
>
> -----Original Message-----
> From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]
> Sent: Wednesday, August 29, 2001 7:58 PM
> To: [EMAIL PROTECTED]
> Subject: Re: (Freeradius 0.1 vs. Freeradius 0.2) + LDAP
>
>
> "Juan Marchionatto" <[EMAIL PROTECTED]> wrote:
> > Hi. I think I may have found a memory leak's cause in the LDAP routines.
>
>   That sounds great!
>
> > The ldap_first_entry routine is reusing the variable 'result', which was
> > previously assigned on ldap_search_st, so any of the ldap_msgfree later
> > calls will not pass the original value, as it should.
>
>   Hmm... from my reading of the ldap docs, that's what it's *supposed*
> to do.  You first search for the results, and then find the first
> entry within that results.
>
> > If this is the problem, using a second (LDAPMessage *) variable for the
> > entry-retrieving calls, should do it.
> >
> > Just as an example, check this code fragment:
> >
> > if ((result = ldap_first_entry(ld_inst, result)) == NULL) {
> >   ldap_msgfree(result);
> >   return 0;
> >  }
> >
> > It is wrong, because you are always passing NULL to ldap_msgfree instead
> > of the original value returned from ldap_first_entry
>
>   Yes, that code is wrong.  But I don't see any code like that in the
> current LDAP module.
>
> > It should be:
> >
> > if ((another_ptLDAPMessage_variable = ldap_first_entry(ld_inst,
> > result)) == NULL) {
> >   ldap_msgfree(result);
> >   return 0;
> >  }
>
>   Yes, that code is correct.  And that's what the current rlm_ldap
> code does.
>
>   Alan DeKok.
>
> -
> 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

Reply via email to