More s3compat-for-review patches

Stefan (metze) Metzmacher metze at
Sat May 29 03:24:14 MDT 2010

Hi Andrew,

>>>>>>> On Wed, 2010-05-26 at 11:45 +1000, Andrew Bartlett wrote:
>>>>>>>> On Wed, 2010-05-26 at 09:54 +1000, Andrew Bartlett wrote:
>>>>>>>>> I've updated my s3compat-for-review tree, which now includes the use of
>>>>>>>>> the source3/ ntlmssp_sign.s and ntlmssp.h in common code.
>>>>>>>>> I would appreciate any useful comments that folks have, as I would like
>>>>>>>>> to merge this code.  Most of the patches should be able to be
>>>>>>>>> cherry-picked on their own, if only some are acceptable.
>>>>>>>> I've updated this branch following feedback on IRC from simo.  (A number
>>>>>>>> of the patches to move functions into different files have been split
>>>>>>>> into multiple patches). 
>>>>>>>> It passes 'make selftest' with the same errors as master.
>>>>>>> If I don't get any further comments (and I believe I've addressed simo's
>>>>>>> concerns about the file split-up being taken any further), I intend to
>>>>>>> push this as-is at the end of this week (tomorrow). 
>>>>>> Please don't push it as is.
>>>>>> I reviewed it yesterday via gitweb on my cellphone (in readonly mode:-)
>>>>>> I'd like to review it more detailed today, as I found some minor details
>>>>>> I'd like to be fixed before it's pushed to master.
>>>>>> The first things I noticed are:
>>>>>> - we don't need a ntlmssp_private.h (at least in this patch stream).
>>>>>> - for "s3:auth Make AUTH_NTLMSSP_STATE a private structure"
>>>>>>   the change to source3/auth/auth_util.c is unrelated and changes
>>>>>>   the behavior. It should be its own commit.
>>>>>>   Would it make sense to handle the case where
>>>>>>   auth_ntlmssp_server_info() is called twice without segfaulting?
>>>>>> - for "ntlmssp Make the ntlmssp.h from source3/ a common header"
>>>>>>   the change to source3/ is completely unrelated.
>>>>>> - for "s3:idmap Use idmap.idl defined structures and constants"
>>>>>>   I'd like to get the ack from Michael as he's currently working on
>>>>>>   idmap changes.
>>>>>>   But for sure deleting source3/librpc/gen_ndr/README is wrong there.
>>>>> I've fixed this problems in my master3-fix branch
>>>> sorry it's
>>> My only comment is that
>>>;a=commitdiff;h=4dd01c2338619828678ceb16e04d1ad7ace3dde0 can either be dropped (I can re-add it when I actually use and test it), or have the commit comment fixed to:
>>> Make it possible to the auth module to provide the full NT token
>>> This allows an auth module to provide all the groups, including the
>>> builtin and local alias groups. 
>> Ok, I'll drop this in my branch and you can readded when you need it.
>>>>> But I need to do some testing and would like to get feedback from
>>>>> at least Michael (idmap) and Günther (auth/ntlmssp).
>>>>> I'm happy to push the patches I have in my master3-reviewed branch
> I think I missed this before.  Please push those.


I pushed updated patches here:;a=shortlog;h=refs/heads/master3-fix
and marked them for additional review (gd, idra and obnox)

I'll fix up the METZE-TODO commits on monday, and then only commit the
other winbind
related commits, better don't try to rebase on this branch as long as
commits are there, as you'll get conflicts...


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <>

More information about the samba-technical mailing list