[PATCH] Explain a bit of how loadparm works

[Volker Lendecke]

On Wed, Oct 16, 2013 at 10:45:58AM +0200, Michael Adam wrote:
> On 2013-10-16 at 10:27 +0200, Michael Adam wrote:
> > On 2013-10-16 at 09:00 +0200, Volker Lendecke wrote:
> > >
> > > One thing sticks out
> > > immediately though is the requirement to use talloc_unlink.
> > > This needs to go ASAP. This is not acceptable. Sorry to be
> > > so strict, but talloc_reference/talloc_unlink is just a
> > > complete no-go in central code. Please focus on removing
> > > that very, very soon.
> > 
> > I absolutely agree generally, but I think talloc_unlink was
> > mentioned here just by habit instead of talloc_free:
> > Looking at the implementation of loadparm_init_s3():
> > 
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > struct loadparm_context *loadparm_init_s3(TALLOC_CTX *mem_ctx,
> > 					  const struct loadparm_s3_helpers *s3_fns)
> > {
> > 	struct loadparm_context *loadparm_context = talloc_zero(mem_ctx, struct loadparm_context);
> > 	if (!loadparm_context) {
> > 		return NULL;
> > 	}
> > 	loadparm_context->s3_fns = s3_fns;
> > 	return loadparm_context;
> > }
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > it seems that talloc_free() woule be safe on the return value, right?
> 
> Oops, apparently not right:
> 
> in lpcfg_gensec_settings(), a
> settings->lp_ctx = talloc_reference(settings, lp_ctx);
> is done...
> 
> We should really get rid of this!

And this is *EXACTLY* the reason why talloc_reference is so
evil and needs to be die, die, die! You fell into the trap
looking at the allocator, and somewhere completely distant
your nice talloc hierarchy is destroyed.

Andrew, please remove the talloc_reference from the loadparm
code as your next priority. Otherwise we will have to work
on unmerging that again.

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de