Ian Puleston wrote:
-----Original Message----- From: Howard Chu [mailto:hyc@symas.com] Sent: Wednesday, June 08, 2011 5:47 PM
I'm pretty sure that this chunk can never do anything useful. ... The connection has just been created, asynchronously, so there's no way that the TLS layer was already started when it got here.
After re-reviewing it I think you are correct. I put that in because I thought there may be a chance that ldap_int_tls_start could get called from ldap_int_open_connection if the connect completed very quickly in the underlying layers. But even if it did there has been no call to ldap_int_poll to check for it having completed. ldap_pvt_connect and then ldap_connect_to_host return -2 to ldap_int_open_connection on an async connect. And the latter then only calls ldap_int_tls_start if rc == 0, so that call will not happen.
Also, I suggest that you only check for CONNST_CONNECTING in the callers, and do the TLS check in the check function
Yes, good idea. I guess that the way I have it allows it to start up TLS immediately if using TLS and the connect completes immediately, but if not using TLS it will return LDAP_X_CONNECTING. What you are suggesting means that if not using TLS and the connect completes immediately then it will be able to go ahead and send the request rather than retuning LDAP_X_CONNECTING.
I will update the fix and supply a new patch shortly.
No need, I already committed a fix to git. It is also in RE24.
Thanks Ian