[PATCH] --option support on Samba3. (was Re: What is lp_load_ex() in source3/param/loadparm.c?)

[Andrew Bartlett]

On Mon, 2010-08-23 at 08:15 +1000, Andrew Bartlett wrote:
> On Sun, 2010-08-22 at 23:31 +0200, Michael Adam wrote:

> > I'd like to go over all callers of lp_load myself and check
> > whether all the parameters to lp_load are correct at all
> > (there may still be regressions introduced by the introduction
> > of registry shares/registry global config), and verify the
> > correct wrappers.
> > 
> > I also think that there are some inaccuracies in the patches
> > changing callers of lp_load to the new wrappers. Some are simply
> > assigned to the wrong commit (not bad but irritating at first
> > sight), and in at least two places you changed a call to
> > "lp_load_initial_only" which does not load registry globals
> > to a full blown lp_load wrapper. I would like to check all
> > the instances. But I don't think I will make it tonight. :-)
> > 
> > I would like to hold off pushing the introduction of the
> > wrappers until I have gone over the callers in detail, if
> > that's ok for you.
> 
> I very much agree.  The call change to lp_load_initial_only() in
> libsmbclient is indeed a bug - sorry about that.  In the first
> generation of this patch I had incorrectly assumed it was equivalent to
> what I later added as lp_load_for_client(), before I re-read your mail. 
> 
> I've just pushed 'loadparm-changes-lp-set-cmdline' to my git repo, in
> case you want to work from there.  I agree it's really hard to stare
> more than a dozen simple replacements and remain convinced that there
> isn't a typo or behaviour change in there, or on the other hand to find
> it!

Did you manage to look at any of these patches?  What do I need to do to
get these changes into the tree?  (Not that they are really important or
earth-shattering, but just because having developed them to learn about
the calling patterns here, I figure it might make it easier for the next
developer to figure it out). 

> > > I've also included the start of an implementation of lp_set_cmdline()
> > > that I've been working on with tridge, in case it can get some comment. 
> > 
> > That looks very interesting and useful, indeed!
> > I guess the lp_set_cmdline() should be integrated in argv parsing
> > in the main functions? The cmdline_override() function in the
> > patch is not used (yet). So I guess the patch is not quite
> > finished yet?
> 
> Correct.  One of the problems at the moment is that we call init_globals
> too often, and so I started to look at the callers to try and understand
> what they wanted.  
> 
> In looking over all the callers, I strongly suspect a lot of copy and
> paste here - for example, winbindd loads the smb.conf with the shares,
> and with IPC$ added.  I can't see any reason why it would need either. 
> 
> If we can understand what the callers want better, we can hopefully
> start to move the lp_load() calls into the popt code for most of the
> utilities like we do in the source4 code.  This could make the
> lp_set_cmdline much easier to implement.  If not, we at least need to
> avoid wiping the table with the initialise_globals parameter after we
> have set the command line options. 

Tridge and I figured out another way to do lp_set_cmdline(), and a new
patch is attached, which I would like reviewed for inclusion into the
tree.  This also includes the hooks into the common popt code, so that
all Samba utilities now have a --option parameter.

Please review,

Thanks,

Andrew Bartlett


-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-param-added-lp_set_cmdline-and-option-parameter.patch
Type: text/x-patch
Size: 11119 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100901/ba59e348/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100901/ba59e348/attachment.pgp>