Re: [PATCH] winbind id assignment module

2002-05-18 Thread Andrew Bartlett

Mike Gerdts wrote:
 
 On Sat, 2002-05-18 at 20:05, Andrew Bartlett wrote:
  I don't think I actually looked at the code for the first patch, only
  the comments.  Unless a patch is *really* big (in which case you should
  consider why this is the case) you would do well to always include the
  patch an either an inline or attached diff, not as a tarball.
 
 I am confused on list etiquette... I thought that samba-technical was
 one that everyone complained about attachments 'cause it screwed up the
 -digest version.

They complain about binary attachments, and html mail.  Patches havn't
caused an issue as far as I know.  Works best if you can convince your
mailer to attach it (as yours did) in a 'text/...' format, as then it
doesn't get encoded etc.

 The patch and a sample module are attached.  The tarball also includes a
 readme, changelog, and a Makefile.

This looks *much* better.

I'm not sure on the 'reload' functionality, but I suppose its a good
idea.  Other than minor things like indenting,   (Try 8-space tabs) I
think this is well on its way to inclusion.

I like the checking of the .so at loadparm time - its a nice touch.

Andrew Bartlett

-- 
Andrew Bartlett [EMAIL PROTECTED]
Manager, Authentication Subsystems, Samba Team  [EMAIL PROTECTED]
Student Network Administrator, Hawker College   [EMAIL PROTECTED]
http://samba.org http://build.samba.org http://hawkerc.net




Re: [PATCH] winbind id assignment module

2002-05-18 Thread Mike Gerdts

On Sat, 2002-05-18 at 20:54, Andrew Bartlett wrote:
 This looks *much* better.
 
 I'm not sure on the 'reload' functionality, but I suppose its a good
 idea.  Other than minor things like indenting,   (Try 8-space tabs) I
 think this is well on its way to inclusion.

The reload functionality was initially intended to be only load().  My
initial look at the existing code suggested that it would get called
again after getting a HUP (I have to look into that though...).  To me
it looked as though it would be easiest just to make it be able to
handle a reload.  I think that it should work as it is, but I haven't
tested it.  The two things that I would want to test are 1) does it do
what you expect, and 2) does it free up all resources related to the
file such that someone debugging a module can count on a day of HUPs
rather than restarts does end up with 50 copies still mapped.

As for spacing... I tried to follow the standard that I saw in the file
already.  I used 4 character tabs, but they should have expanded out
OK.  I thought that others were using 4 character tabs as well because
sections of winbindd_idmap.c have tabs expanded to four characters.  In
any case, the next version that goes out will get rid of any expanded
tabs and any necessary reformatting for prettiness will take place.

 I like the checking of the .so at loadparm time - its a nice touch.

Thanks!

Mike