Two diffs to add paramater self-checking

[David Collier-Brown]

Andrew Bartlett wrote:
> 
> David Collier-Brown wrote:
> >
> > These are for param/loadparm.c and utils/testparm.c, respectively,
> > to put in the self-checking that's been languishing...
> 
> My comments apply to HEAD, btw - which branch was the patch intended
> for?

	I developed it at home, so it was something like
	2_2_STABLE. Before submitting, I applied my changes
	to 2.2.5 production and send diff -u output from 
	that.

	If this is too different, I can set up CVS from
	home for whatever branncg you prefer.

> 
> > +   /* Security = (server or domain) requires password server to be set.  */
> > +   if ((Globals.szPasswordServer == NULL || Globals.szPasswordServer[0] == '\0'
> > +)
> > +     && (Globals.security == SEC_SERVER || Globals.security ==
> > +SEC_DOMAIN)) {
> > +      DEBUG(0,("ERROR: security = server and security = domain "
> > +         "both require a password server.\n"));
> > +      bRetval = False;
> > +   }
> 
> No - when security = domain, it will try to find a DC if password server
> is not set.

	Thanks, I didn't know that it could do that!

> 
> > +   /* Password server should be a netbios name. */
> > +   if (Globals.szPasswordServer != NULL
> > +     && (strchr(Globals.szPasswordServer,'.') != NULL
> > +       || strlen(Globals.szPasswordServer) >= 15)) {
> > +       DEBUG(0,("ERROR: password server \"%s\" is not a legal "
> > +                "NetBIOS name, logons will fail.\n",
> > +                Globals.szPasswordServer));
> > +       bRetval = False;
> > +   }
> 
> No, we accept IP addresses too.  

	Ok, will allow both.

> 
> > +   /* Check unix password sync prerequisites. */
> > +   if (Globals.bUnixPasswdSync) {
> > +      if (Globals.security != SEC_USER) {
> > +         DEBUG(0,("WARNING: unix password sync = yes requires "
> > +           "security = user.\n"));
> > +      }
> 
> No - we can have local accounts when secuirty=domain, and we should be
> able to sync on those accounts.

	Ok, will allow both...


> 
> > +      if (Globals.bEncryptPasswords == False) {
> > +        DEBUG(0,("WARNING: unix password sync = yes requires "
> > +           "encrypt passwords = yes.\n"));
> > +      }
> > +   }
> 
> No - if encrypt passwords = no, then without this no password change at
> all is done.

	Er, that's what I was warning about...


 
> > +      else if ((buf.st_mode & S_IFDIR) != S_IFDIR) {
> > +         DEBUG(0,("ERROR: Path %s in service [%s] isn't a directory.\n",
> > +               s->szPath, s->szService));
> > +      }
> > +   }
> 
> Watch out that people use a pile of % macro substituions here - which
> could make this a false alarm...

	Yes, it could require being fully expanded before the
	check is made.  Thanks!

 
> > +   if (s->bBrowseable == False && Globals.bBrowseList == True
> > +     && strwicmp(s->szService,HOMES_NAME) != 0) {
> > +      DEBUG(0,( "NOTICE: Service [%s] is unbrowsable, but browse "
> > +           "lists are being served.\n", s->szService));
> > +   }
> 
> Isn't this a quite standard setting for the [homes] share?

	Yes: it's special-cased above: I was generating
	bogis messages on my own system until I did that.

 

> 
> > +       if (lp_wins_support() && *lp_wins_server()) {
> > +               printf("ERROR: both 'wins support = true' and 'wins server = <server>' \
> > +cannot be set in the smb.conf file. nmbd will abort with this setting.\n");
> > +               ret = 1;
> > +       }
> 
> Probably valid, but we might want to consider wins replication here.
> Also, in HEAD tridge allows muliple wins server groups - where this
> would be quite valid.

	Good: I do expect that some of these checks will
	get taken out as the program becomes more general.


--dave
-- 
David Collier-Brown,           | Always do right. This will gratify 
Performance & Engineering      | some people and astonish the rest.
Americas Customer Engineering, |                      -- Mark Twain
(905) 415-2849                 | davecb at canada.sun.com