[389-devel] Please review (take 3): [389 Project] #402: unhashed#user#password in entry extension
Thanks to Nathan for his reviews and suggestions. I fixed the previous
proposal following his advice.
https://fedorahosted.org/389/ticket/402
https://fedorahosted.org/389/attachment/ticket/402/0001-Trac-Ticket-402-n...
(This patch includes including autogen'ed files.)
Thanks,
--noriko
389 Project wrote:
Replying to [comment:5 nkinder]:
> There appears to have been a problem merging your changes in
ldap/servers/slapd/ldaputil.c with the patch for ticket #399. Your patch
reverts the fix made to always get the bind result, which will cause a
regression.
Sorry, I don't know why the file was reverted. I recreated the patch
which has no change on ldaputil.c.
> It looks odd that the slapi_entry_apply_mod_extension() and
entry_apply_mod_wsi() functions expect SLAPI_PW_* specific extensions
since these are generic functions. Is this a safe assumption?
Yes, you are right! I replaced SLAPI_PW_SET_ADD with SLAPI_EXT_SET_ADD
and SLAPI_PW_SET_REPLACE with SLAPI_EXT_SET_REPLACE.
> If pw_entry_constructor() fails to allocate a lock, should we just
return NULL without allocating and returning pw_extp? The slapi_rwlock_*()
functions called by the extension get/set functions do check if the lock
is NULL before attempting to use them, but this will behave with no
protection from the lock.
A good point. Thanks for pointing it out. I've modified constructor to
return NULL when slapi_new_rwlock fails. If it happens, "struct
slapi_pw_entry_ext" won't be allocated. And the following unhashed
password set/get fails with ""pw_entry_extension is not set".
> Does pw_copy_entry_ext() need to use the rwlock from the source and
destination extensions?
Another good point! I added to call slapi_rwlock_wrlock to protect
valuearray_add_valuearray.