[PATCHES] Merge loadparm further

[Andrew Bartlett]

On Fri, 2014-06-27 at 11:00 +1200, Garming Sam wrote:
> Regarding any work further from this point, it has become increasingly 
> apparent that the
> approach of passing around pointers and the use of the s3 helper 
> functions isn't really the
> right approach.
> 
> Fundamentally, the issue lies with the life of the temporary contexts. 
> Their purpose was to be
> be temporary, but they point to global variables which are longer lived 
> and change. I first noticed
> the issue when attempting to merge 'add a service', and realized how the 
> service pointers could be
> realloc'd beneath it.
> 
> The work up to this point should not be affected, but I don't think the 
> problem can continue
> to be sidestepped. At least, not if there is ever to be a single 
> well-integrated loadparm
> system. It really needs an overhaul in how the two interact, which can't 
> be achieved bit by bit.
> This would probably involve lifting the loadparm context up and making 
> it global in lib/param.
> At one point, a little while back, I took a brief crack at this and ran 
> into a number of initialization issues
> and implicit assumptions, with fields being set to zero for instance. 
> These kinds of issues would need
> to be dealt with.
> 
> 
> It would be nice to hear any thoughts on this.

What about this as a plan:

Rewrite loadparm_init_s3() to be more like loadparm_init_global(), that
is it returns a global pointer.  Change the talloc_zero() to be a
loadparm_init() call. 

We would need to remove the mem_ctx argument and audit callers to remove
talloc_steal() and talloc_free(), but fortunately the pattern I
established was for talloc_unlink(), which will safely fail.

Then rather than replacing loadparm_context->globals, we use it, along
with the lp_ctx->services pointer, by no longer having a special handler
for do_section and do_parameter.  We would need to merge the
hash_a_service code, but that shouldn't be too hard.

We then remove all the other references to &Globals (each lp function
for example, by using the new loadparm_init_s3()).  For example, the
lp_string_set stuff, and then chase down any initialisation issues.

If we can't do that much at once, then at least replace lp_do_section,
by keeping &Globals but removing the ServicePtrs.  You could start that
process by creating a helper function that returns the current, correct
ServicePtrs from the global in loadparm_init_s3(). 

I do really appreciate you taking on this massive challenge, and all the
work you have done on it.

I hope this helps,

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba