[PATCH] Make loadparm more common

Andrew Bartlett abartlet at samba.org
Tue Apr 1 17:05:05 MDT 2014


On Tue, 2014-04-01 at 10:06 +0200, Andreas Schneider wrote:
> On Tuesday 01 April 2014 17:21:02 you 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=5fa1a3cd62e0dce
> > fc5364f83046db025fc0e65b9
> 
> -       if ((iTemp = getservicebyname(pszParmValue, &serviceTemp)) >= 0) {
> +       if ((iTemp = getservicebyname(pszParmValue, serviceTemp)) >= 0) {
> 
> Please split up those lines in two:
> 
> 	iTemp = getservicebyname(pszParmValue, serviceTemp)
> 	if (iTemp >= 0) {

This is gone by the end of the patch series.  Can we leave it as-is
rather than churn it at this point?

> > http://git.catalyst.net.nz/gitweb?p=samba.git;a=commitdiff;h=909fb5a6c7df2e
> > 2cc7e48d5814eb2cff53ee6664
> 
> The change is ok, but if the context is already a talloc pool why not get rid 
> of string_init() completely and use always talloc_strdup()?

That is the eventual goal.  This is one (very) small step towards that.
The later commit is:

commit 38abab10e729c6bfeb18f608b29754e9e3e6534e
Author: Garming Sam <garming at catalyst.net.nz>
Date:   Thu Feb 27 12:14:57 2014 +1300

    param: remove string_init and inline it into string_set
    

> > http://git.catalyst.net.nz/gitweb?p=samba.git;a=commitdiff;h=5c9f5892c4e330
> > 4e2e3f11be0a41d087f9df6d9c
> 
> I think here you should allocate with talloc directly on the Globals.ctx and 
> not do it on talloc_tos(). string_init() dupclicates the memory and then you 
> free it.

The aim is to do one small step at a time here, and this keeps it
consistent for the time-being. 

> > http://git.catalyst.net.nz/gitweb?p=samba.git;a=commitdiff;h=746cbfcd93c38e
> > 5bb66e892fa2ef0d0d11888703
> 
> This one looks fine.

I'll add your review once I'm confident that Volker's query has been
addressed.

Thanks!

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






More information about the samba-technical mailing list