Yoshinori Nishino wrote: > Dear Howard-san, >=20 >=20 > Thanks to your advice, I could create the attached tentative patches.
Thanks, this looks much better. >=20 > They modifies servers/slapd/passwd.c and configure.in > so that running "./configure" can check whether or not crypt_r() exists > and use the function in slapd_crypt() if possible. >=20 > I am going to check whether or not "calloc()/free()" causes > unacceptable=E3=80=80memory fragmentation. Never use calloc/malloc/free directly in slapd code. Always use=20 ch_calloc/ch_malloc/ch_free. In this particular case, there's no reason to allocate at all. Just defin= e=20 "struct crypt_data data" as a local variable. >=20 >=20 > 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 the >>> 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: >>>> [email protected] 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 becom= e >>>>> 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 sla= pd? >>>> >>>> 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 that >>>> 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/
