Howard Chu wrote:
Yoshinori Nishino wrote:
Dear Howard-san,
Thanks to your advice, I could create the attached tentative patches.
=20 Thanks, this looks much better.
They modifies servers/slapd/passwd.c and configure.in so that running "./configure" can check whether or not crypt_r() exist=
s
and use the function in slapd_crypt() if possible.
I am going to check whether or not "calloc()/free()" causes unacceptable=E3=80=80memory fragmentation.
=20 Never use calloc/malloc/free directly in slapd code. Always use=20 ch_calloc/ch_malloc/ch_free. =20 In this particular case, there's no reason to allocate at all. Just def=
ine=20
"struct crypt_data data" as a local variable.
Also there's no reason to lock the passwd_mutex at all. That would comple= tely=20 defeat the purpose of using crypt_r() in the first place.
Best Regards,
On 2017/09/01 20:23, Yoshinori Nishino wrote:
On 2017/08/30 20:46, Yoshinori Nishino wrote:
Dear Howard-san,
Thank you so much for your prompt advice.
As you told, I need to check whether or not crypt_r() exists. I am going to consider including the check.
Would it be possible to let me know whether there are any other concerns about the fix if we can use crypt_r()?
The implementation that "both calloc() and free() occur every time slapd_crypt() is called" does not seem to look good. The callers of slapd_crypt() might be able to get the memory for "struct crypt_data" before they call it. However, I think it causes the changes of other source codes. So, I think the aforementioned implementation is a workaround for th=
e
slowdown if the risk of memory fragmentation can be acceptable.
On the other hands, I think we do not need to care about strcmp() because it is thread-safe.
Best Regards,
On 2017/08/30 19:26, Howard Chu wrote:
yos-nishino@ys.jp.nec.com wrote:
Full_Name: Yoshinori Nishino Version: 2.4.45 OS: CentOS 7 URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (210.143.35.20)
Dear sir,
The function slapd_crypt() in servers/slapd/passwd.c seems to beco=
me
slow when many ldap client connections occur. It seems it is because the function uses crypt()(non thread-safe function) and pthread_mutex_lock(), which results in the slowdown. #Besides, we need to use {CRYPT} hash as users' password hash.
So, I modified servers/slapd/passwd.c like the following. As a result, slapd_crypt() becomes much faster under the same condition. Would you let me know whether or not the fix is appropriate for sl=
apd?
No it is not an appropriate fix.
You should add an autoconf test to check for the existence of the crypt_r function, and use an #ifdef here based on the result of tha=
t
test, since crypt_r is a non-standard function.
=20 =20
--=20 -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/