More s3compat-for-review patches

[Stefan (metze) Metzmacher]

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.
>>>>>>>>>
>>>>>>>>> http://gitweb.samba.org/?p=abartlet/samba.git/.git;a=shortlog;h=refs/heads/s3compat-for-review
>>>>>>>>
>>>>>>>> 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/Makefile.in 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
>>>>> http://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/head/master3-fix
>>>>
>>>> sorry it's
>>>> http://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-fix
>>>
>>> My only comment is that
>>> http://gitweb.samba.org/?p=metze/samba/wip.git;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
>>>>> http://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-reviewed
> 
> I think I missed this before.  Please push those.

Done.

I pushed updated patches here:
http://gitweb.samba.org/?p=metze/samba/wip.git;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
the METZE-TODO
commits are there, as you'll get conflicts...

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/20100529/59cd339c/attachment.pgp>