[PATCH] Make loadparm more common

Volker Lendecke Volker.Lendecke at SerNet.DE
Tue Apr 1 23:03:28 MDT 2014


On Wed, Apr 02, 2014 at 11:43:13AM +1300, Andrew Bartlett wrote:
> On Tue, 2014-04-01 at 08:43 +0200, Volker Lendecke wrote:
> > On Tue, Apr 01, 2014 at 05:21:02PM +1300, Andrew Bartlett wrote:
> > > At this point, what I would like to see is these patches reviewed:
> > > http://git.catalyst.net.nz/gitweb?p=samba.git;a=commitdiff;h=5fa1a3cd62e0dcefc5364f83046db025fc0e65b9
> > 
> > This has a talloc_zero(NULL,...). Would it be possible to
> > use talloc_tos() here?
> 
> No, I'm not willing to add any more talloc_tos() to this area of the
> code.  Almost all the odd unexpected failures caused by this patch set
> were due to new talloc_tos() calls, because not all callers had a
> talloc_stackframe().  

talloc_tos() should not fail if there is no stackframe
around. It should just print a warning. Can you explain in
more detail how this fails?

> Additionally, it would have to be un-done again before the code is made
> common later in the series, and we don't use talloc_tos() in the top
> level code. 

Then new code should take a talloc context, so that the
caller can pass a talloc_tos(). talloc(NULL,...) is pretty
much a no-go these days in new code.

> > > http://git.catalyst.net.nz/gitweb?p=samba.git;a=commitdiff;h=746cbfcd93c38e5bb66e892fa2ef0d0d11888703
> > 
> > Just for simplicity, can we make it such that the equivalent
> > piece of code in source3/param/loadparm.c looks exactly the
> > same? If we touch the code, we should unify the code lines
> > 1:1.
> 
> I'm a little confused, as that is what is being done here, but just
> isn't all done in this commit.  Eventually they become identical, then
> merge. 

Garmings patch has

for (data = pserviceSource->param_opt; data != NULL; data = data->next) {

whereas the source3 copy has

data = pserviceSource->param_opt;
        while (data) {

The for loop is arguably better, but it is syntactically
different, so it will cause someone to look at the code
later again. I just thought it might be good to make this
syntactically the same now that someone looks at the code
anyway. True, this is a minor nit-pick that I possibly
should not have brought up to not waste bandwidth, sorry for
that.

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


More information about the samba-technical mailing list