[PATCH] Make loadparm more common

Garming Sam garming at catalyst.net.nz
Thu Apr 10 17:32:33 MDT 2014


On 02/04/14 11:43, 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?
This talloc actually gets removed a few patches later. I've just 
squashed these
two patches together now.

There's one other place,  get_parametrics_by_service, which now uses a 
talloc_stackframe.

> http://git.catalyst.net.nz/gitweb?p=samba.git;a=commitdiff;h=909fb5a6c7df2e2cc7e48d5814eb2cff53ee6664
>> While in general a good direction, this one has the snippet
>>
>> -                               SAFE_FREE(f->subfname);
>> -                               f->subfname = SMB_STRDUP(n2);
>> +                               TALLOC_FREE(f->subfname);
>> +                               f->subfname = talloc_strdup(f, n2);
>>                                  TALLOC_FREE(n2);
>>
>> SMB_STRDUP panics on failure, talloc_strdup does not. So we
>> need NULL checks in these scenarios. I haven't checked all
>> places where this is done.

In the places where there was an SMB_STRDUP or similar call, there
should now be panics and debugs. Notably in the lib/param code, there's
a number of places where it does neither in exactly the same places. In
such places, I've just put a debug instead of a panic since many of
those places get merged fairly soon after (and that would preserve the
lib/param behaviour).

Is there a preference in a panic or a debug?


Branch here:

git://git.catalyst.net.nz/samba.git       loadparm-talloc-to-review5

http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/heads/loadparm-talloc-to-review5

I've run a make test and it appears to pass all the tests.


I really appreciate that people do want to see this code changed. Just 
working with it,
I've managed to come across some real subtleties in the code and between 
the two
loadparms. It's taken lots of little changes to fix some these, many of 
which get nullified a
little while later. But without these minor changes, it really would be 
difficult to not get it wrong.



Thanks,

Garming Sam



More information about the samba-technical mailing list