Re: (ITS#6788) Only one reference when ldap_open_internal_connection

[hyc@symas.com]

masarati@aero.polimi.it wrote:
>> Full_Name: Jiri Giesl
>> Version: 2.3.43
>> OS: Linux
>> URL: ftp://ftp.openldap.org/incoming/
>> Submission from: (NULL) (212.111.5.220)
>>
>>
>> We tried to estalish connection to LDAP server by our own (because of
>> proxy
>> usage) and then use "ldap_open_internal_connection" function to initialize
>> internal structures of openldap. After successful SASL binding we realized
>> that
>> the connection to LDAP server has been closed. We found out that there is
>> checking for reference count in "wait4msg" function (result.c file) this
>> way:
>>
>> if ( lc->lconn_refcnt<= 1 )
>>
>> During classic initialization and SASL binding we get 2 references,
>> however when
>> using "ldap_open_internal_connection" and SASL binding we get only 1
>> reference.
>>
>> We realized that in "ldap_open_defconn" (function which is called for
>> establishing of a new connection) there is reference counter which is
>> increased
>> this way:
>>
>> ++ld->ld_defconn->lconn_refcnt;	/* so it never gets closed/freed */
>>
>> When we added this incrementation also into
>> "ldap_open_internal_connection"
>> (open.c file), everything goes well (it is obvious).
>>
>> Are we wrong or this is really minor bug (missing incrementation of
>> reference
>> counter in "ldap_open_internal_connection")?
>
> Your comment sounds fine; however, it's hard to judge: that code is
> essentially untouched since 2000 (Mark Adamson) and it is not (or no
> longer) used elsewhere in OpenLDAP code.  I'd vote for adding a
>
>      ++ld->ld_defconn->lconn_refcnt;
>
> here as well, unless someone else has valid objections.

I'd say they should rewrite their code to use ldap_init_fd() instead; since 
open_internal_connection does not provide a URL some SASL mechanisms would 
fail because they don't know the remote hostname.

But yeah, probably adding the refcnt is OK.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/