Of challenges in loadparm

[Andrew Bartlett]

I recently had a good chat with Michael Adam <obnox at samba.org> on IRC
about some of the goals, directions and challenges in Samba's loadparm
code, and so I wanted to write some of my thoughts down, so we could
discuss things on the list, and so the discussion can be found by Google
later :-)

In this mail, I'm speaking particularly about the loadparm code in
source3/param.  The soruce4/ loadparm code has a similar structure, but
has solved some of these issues by loosing features. 

Samba's loadparm has grown over time, and so has a number of features
that have become embedded into Samba's expected behaviours, but which
make it harder to implement in a layered fashion.  

In particular this is a product of the autoconf/makefile object-list
based build system, because without the restrictions placed by fully
defined libraries, there is little to prevent the programmer
implementing layering violations.  There has also been little need to
force a consistency in implementation, because the original structure
was so flexible.

In the past few months, I've been working to put the 'straight-jacket'
back on loadparm.  I've removed direct assignment of global variables
(must go via a .special handle routine instead), and ensured that every
parameter sets a variable in the structure.  I've removed the separate
variables for the global_myname() and global_myworkgroup(), instead
storing these directly in loadparm, and using the lp_set_cmdline() code
to allow overrides where required.  I've also removed the reference to
lp_string() (and the substitution code it relies on) for a number of
parameters, and added a way to get at the s3 loadparm in a 'struct
loadparm_context' expected by the soruce4 code. 

I've also automated the generation of the struct loadparm_global and
struct loadparm_service structures.  This has reduced the number of
places that a parameter is declared, but as Michael pointed out on IRC,
there is much more to do.  We previously declared a parameter in:

 - the docs (including the default value)
 - the parm_struct table (the big C99 table of parameters)
 - the struct loadparm_service or struct loadparm_globals
 - the sDefaults and init_globals()
 - proto.h

As I mentioned, we now auto-generate struct loadparm_service/struct
loadparm_globals, which did made it much easier to combine the service
paramters, but moving from 5 to 4 declarations isn't a solution, it's
just one small step.  

As another small step, I would propose that we take the next step (taken
in source4 for some time) and generate the lp_ prototypes into a
param_proto.h. 

We should also rename all the variables in struct loadparm_service and
struct loadparm_globals to match exactly the lp_ functions that will
access them, and finally rid us of the 'Hungarian notation' variable
names.

However, the correct, long term fix is indeed to generate all these from
a single table, preferably with some connection to the documentation.
The challenge here is that we still have very many special cases in our
loadparm code - a large number of lp_ functions are not in fact
generated, but are hand-coded to return different values in particular
circumstances.  Each special case works against useful auto-generation,
and we should look carefully to see if we can handle them in another
way. 

We also discussed the idea of introducing much more layering to the
loadparm code, creating a distinct 'get' and 'set' layer, and possible
first parsing the smb.conf, then implementing it. 

The history of loadparm presents a number of challenges in the
introduction of layering, because a number of parameters impact on the
loading of the smb.conf file, or the display of error messages from that
load process.  So, for example:
                DEBUG(3, ("Processing section \"[%s]\"\n",
pszSectionName));
This is shown at a debug level above the default, so only appears if the
debug is set on the command line, or earlier in the smb.conf.  This
works because the .special handler for 'log level' causes this to be
acted on during the parse.  

Similarly, the 'config file' and 'include = ' directives are meta
directives, which need to be acted on as part of the parse - the
'include = registry' in particular introducing the full complexity of
the registry TDB load to the parsing.  (This is also a source of
dependency issues).

The other big challenge is the much-used % macros, which are embedded
strongly as one of the power features of Samba that our most advanced
(and vocal) users rely on for Samba's full flexibility.  'include = %
m.smb.conf' is a popular approach (I've certainly used it myself).  This
similarly violates the parse/load/read layers, because it needs to know
the machine name for %m.

One layer that can be split with reasonable ease is the 'get' and 'set'
layers.  All parameters (even registry parameters) are written into the
struct loadparm_globals and struct loadparm_service with do_parameter(),
and the lp_*() functions rely only on the lp_string() and these
structures.  As such, these could be moved into another file and
subsystem, potentially breaking some of our last dependency loops. 

Andrew Bartlett
-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org