On Tue, 2013-04-02 at 10:21 +0200, Lukas Slebodnik wrote:
On (29/03/13 09:59), Simo Sorce wrote:
>On Fri, 2013-03-29 at 11:29 +0100, Lukas Slebodnik wrote:
>> On (25/03/13 12:44), Jakub Hrozek wrote:
>
>> >Nack, I'm getting segfault on login with these patches, backtrace is
>> >pointing to sss_authtok_copy
>>
>> Problem was that "struct pam_data" was allocated on stack and zero
initialized.
>> Now this case is correctly handled in copy_pam_data.
>>
>> Changes to previous verion of patches:
>> --only in patch 0002-Making-the-authtok-structure-really-opaque.patch
>> in file src/providers/dp_pam_data_util.c
>> in function copy_pam_data
>>
>> Patches attached.
>
>
>Hey Lukas,
>given you now made authtok an actual talloc context in all cases, I
>think it would make sense to drop the mem_ctx argument from all the
>sss_authtok_set_*() calls. and pass only a sss_auth_tok pointer, and
>document it must be allocated with sss_authotk_new in authtok.h
TALLOC context removed from copy and setter functions.
There isn't other way how to create new authtok like using function
sss_authtok_new and it has already been documented.
>
>Also I would rename sss_authtok_new_empty, to just sss_authotk_new,
>whether by default it is empty or not is an internal detail and the
>suffix '_new' is generally explicit enough.
Renamed.
>
>Simo.
>
New patches attached.
Looks good to me, if it passes all tests it's an ACK.
Simo.
--
Simo Sorce * Red Hat, Inc * New York