More s3compat-for-review patches
Andrew Bartlett
abartlet at samba.org
Fri May 28 17:20:55 MDT 2010
On Thu, 2010-05-27 at 16:55 +0200, Stefan (metze) Metzmacher wrote:
> 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.
Thanks,
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100529/32fde887/attachment.pgp>
More information about the samba-technical
mailing list