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 like this commits:
"s3:winbind tidy up connecting the winbind sockets."
"s3:winbind Avoid use-after-free in s3compat wrapper of winbindd"
but we should defer the code moving in winbindd to happen after this
changes.

I write more comments later...

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/20100527/0ff9bccb/attachment.pgp>