[SCM] Samba Shared Repository - branch master updated

[Stefan (metze) Metzmacher]

Am 18.10.2011 18:52, schrieb Jeremy Allison:
> On Tue, Oct 18, 2011 at 09:49:41AM -0700, Jeremy Allison wrote:
>> On Tue, Oct 18, 2011 at 01:55:04PM +0200, Andrew Bartlett wrote:
>>> The branch, master has been updated
>>>        via  94799db s3-auth move the s3 auth context onto gensec_ntlmssp once we start
>>>        via  fa12756 s3-libsmb Use a gensec module to provide the ntlmssp client in ntlmssp_wrap.c
>>>        via  f9b0426 s3-ntlmssp split auth_ntlmssp_client_start() into two parts
>>>        via  f3333bd s3-rpc_client remove cli_auth_ntlmssp_data_destructor
>>>       from  56328a4 s3/doc: add man page for aio_fork vfs module
>>>
>>> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
>>>
>>>
>>> - Log -----------------------------------------------------------------
>>> commit 94799db9b5d33ded34ad3e934da673a44d48094a
>>> Author: Andrew Bartlett <abartlet at samba.org>
>>> Date:   Tue Oct 18 16:34:27 2011 +1100
>>>
>>>     s3-auth move the s3 auth context onto gensec_ntlmssp once we start
>>>     
>>>     We do not need it on the auth_ntlmssp_state any longer.
>>>     
>>>     Andrew Bartlett
>>>     
>>>     Autobuild-User: Andrew Bartlett <abartlet at samba.org>
>>>     Autobuild-Date: Tue Oct 18 13:54:36 CEST 2011 on sn-devel-104
>>>
>>> commit fa1275610b3c7cad75b5b86ae4b32d8781d1acc0
>>> Author: Andrew Bartlett <abartlet at samba.org>
>>> Date:   Tue Oct 18 16:16:02 2011 +1100
>>>
>>>     s3-libsmb Use a gensec module to provide the ntlmssp client in ntlmssp_wrap.c
>>>     
>>>     This removes the need to have if (ans->gensec_security) everywhere.
>>>     
>>>     Andrew Bartlett
>>
>> Andrew,
>>
>> 	I *really* *really* hate the use of talloc_reference()/talloc_unlink()
>> in this code. I know it's essentially doing the same as a talloc_move()
>> in this case, but it doesn't always do so (looking at the calls to
>> gensec_set_credentials() in source4/smb_server/smb/negprot.c for
>> example.
>>
>> I really think that doing references in gensec is a completely broken
>> design that *WILL* come back and bite us sometime later.
>>
>> Is there some way gensec can be changed to make a copy of
>> the credentials instead ?
>>
>> It's probably not possible to fix this short term, but I'd
>> strongly recommend gensec moving away from talloc_reference()
>> of any passed in pointers. This is a disaster waiting to
>> happen IMHO.
> 
> Being more constructive :-).
> 
> talloc_reference() is only used inside gensec_start.c, so
> we only have a small piece of code to fix (and all the
> callers of course :-).
> 
> I'd be willing to work on this if you agree.

As a long term goal I think we should avoid talloc_reference()
and just let the caller to keep stuff arround for the lifetime
of the created object (or async _send()/_recv() function function pair,
all tevent_req based functions I know about follow that rule).
As the caller is the one should now about the lifetimes of complex
structures.
For simple things like strings we should just copy.

Or we should have explicit donations of objects like here:

NTSTATUS wrepl_socket_donate_stream(struct wrepl_socket *wrepl_socket,
                                    struct tstream_context **stream)
{
        if (wrepl_socket->stream) {
               return NT_STATUS_CONNECTION_ACTIVE;
        }

        wrepl_socket->stream = talloc_move(wrepl_socket, stream);
        return NT_STATUS_OK;
}

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20111019/86ba1301/attachment.pgp>