Options/arguments smbd, nmbd winbindd

[David Collier-Brown]

   This is actually an instance of the "controlled change" problem, one
where users need to take action, in this case by changing the smb.conf
file, to adapt to a change.

   There are two discussions to have here:
1) do we wish to make the change, and
2) how do we wish to deal with users not changing,
and we need to balance **both** of them with one another.

   The current behavior is to ignore the error when faced with
and unexpected or misspelled option.
   The user is not notified, so they don't have the chance to
fix it.
   Let's assume that's a bad thing (;-)) and that we want to fix it.

   We now face the user with a task: to fix this newly-reported error.
If we merely warn about it, there is a good chance the warning
will be ignored.  If we exit, the user will not notice until they
start smbd, which may either be at install time or the next reboot.
   If it's at install time, and if and only if they try a test,they
have a good chance to notice it, read the log and fix the error.
   If it's at boot time, they have a very good chance of not
noticing until a client can't get to their data.

   Balancing the latter case against the original bug, it's arguably
worse to have fixed the bug: we produce silent failures (:-()

   If we only warn about it, there is a chance of it being corrected,
but no chance of a silent failure of smbd.

   I'd claim that this means we should warn about it, **in this case**.
If we were introducing a new option with the ability to cause a
silent failure if mis-used, the discussion might well go a different
direction.

--dave

Karolin Seeger wrote:
> Hi list,
> 
> smbd, nmbd and winbindd can be started with invalid options currently. 
> The first patch attached would be a possible solution.
> It contains an exit if an invalid option has been used. The main problem
> is, that existing setups with wrong options or missing arguments in start
> scripts will break (which is the right behaviour from my point of view).
> 
> The second patch attached prints a warning if an invalid option is used.
> The daemon starts despite the warning. There will be no warning in the
> case of missing arguments (e.g. smbd -s without path to config file).
> (The patch contains only the smbd as an example. This would be the same for
> nmbd and winbindd.)
> 
> What do you think is the right solution?
> Are there any other suggestions?
> 
> Cheers,
> Karo
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Index: smbd/server.c
> ===================================================================
> --- smbd/server.c	(revision 24485)
> +++ smbd/server.c	(working copy)
> @@ -856,6 +856,10 @@
>  			build_options(True); /* Display output to screen as well as debug */ 
>  			exit(0);
>  			break;
> +		default:
> +                        d_fprintf(stderr, "\nInvalid option %s: %s\n",
> +                                 poptBadOption(pc, 0), poptStrerror(opt));
> +                        exit(1);
>  		}
>  	}
>  
> Index: nsswitch/winbindd.c
> ===================================================================
> --- nsswitch/winbindd.c	(revision 24485)
> +++ nsswitch/winbindd.c	(working copy)
> @@ -1012,10 +1012,16 @@
>  
>  	/* Initialise samba/rpc client stuff */
>  
> -	pc = poptGetContext("winbindd", argc, (const char **)argv, long_options,
> -						POPT_CONTEXT_KEEP_FIRST);
> +	pc = poptGetContext("winbindd", argc, (const char **)argv, long_options, 0);
>  
> -	while ((opt = poptGetNextOpt(pc)) != -1) {}
> +	while ((opt = poptGetNextOpt(pc)) != -1) {
> +		switch (opt) {
> +                default:
> +                        d_fprintf(stderr, "\nInvalid option %s: %s\n",
> +                                 poptBadOption(pc, 0), poptStrerror(opt));
> +                        exit(1);
> +                }
> +	}
>  
>  	if (server_mode == SERVER_MODE_INTERACTIVE) {
>  		log_stdout = True;
> Index: nmbd/nmbd.c
> ===================================================================
> --- nmbd/nmbd.c	(revision 24485)
> +++ nmbd/nmbd.c	(working copy)
> @@ -652,6 +652,7 @@
>  	BOOL no_process_group = False;
>  	BOOL log_stdout = False;
>  	enum smb_server_mode server_mode = SERVER_MODE_DAEMON;
> +	int opt;
>  
>  	struct poptOption long_options[] = {
>  	POPT_AUTOHELP
> @@ -674,7 +675,14 @@
>  	global_nmb_port = NMB_PORT;
>  
>  	pc = poptGetContext("nmbd", argc, argv, long_options, 0);
> -	while (poptGetNextOpt(pc) != -1) {};
> +	while ((opt = poptGetNextOpt(pc)) != -1) {
> +		switch (opt) {
> +		default:
> +                        d_fprintf(stderr, "\nInvalid option %s: %s\n",
> +                                 poptBadOption(pc, 0), poptStrerror(opt));
> +                        exit(1);
> +		}
> +	};
>  	poptFreeContext(pc);
>  
>  	global_in_nmbd = True;
> 
> 
> ------------------------------------------------------------------------
> 
> Index: smbd/server.c
> ===================================================================
> --- smbd/server.c	(revision 24485)
> +++ smbd/server.c	(working copy)
> @@ -856,6 +856,9 @@
>  			build_options(True); /* Display output to screen as well as debug */ 
>  			exit(0);
>  			break;
> +		case POPT_ERROR_BADOPT:
> +			d_fprintf(stderr, "Unknown option found!\n");
> +			break;
>  		}
>  	}
>  

-- 
David Collier-Brown,         | Always do right. This will gratify
System Programmer and Author | some people and astonish the rest
davecb at spamcop.net           |                      -- Mark Twain
(416) 223-5943