[PATCH] Merging loadparm further

Garming Sam garming at catalyst.net.nz
Wed Jul 30 20:48:20 MDT 2014


Thanks a lot Michael.

I've looked over the changes you've made and they all seemed pretty 
reasonable.


Cheers,

Garming

On 29/07/14 22:58, Michael Adam wrote:
> Hi Garming,
>
> I am now done reviewing your patches.
> Attached find my reviewed version. I:
> - adapted a few commit messages
> - fixed one formatting
> - split one commit in 2
> - replaced your comment fix by my comment removal
> - added one loadparm patch on top
>
> I indicated in the commit message by "(OBNOX:...)" where I changed
> something.
>
> I'm happy with thos changes pushed to master.
> My two (trivial) patches would need a team-review.
>
> The patches can also be found in my master-loadparm branch:
>
> git://git.samba.org/obnox/samba/samba-obnox.git
>
> https://git.samba.org/?p=obnox/samba/samba-obnox.git;a=shortlog;h=refs/heads/master-loadparm
>
> Cheers - Michael
>
>
> On 2014-07-26 at 02:20 +0200, Michael Adam wrote:
>> Hi Garming,
>>
>> nice progress!
>>
>> On 2014-07-25 at 14:33 +1200, Garming Sam wrote:
>>> Hi there,
>>>
>>> I've run these patches through the usual tests and made sure each
>>> patch passes the main tests we identified. It also passes the full
>>> autobuild with the winbindd patches in my other mail.
>>>
>>> This set of patches actually makes some good progress in merging the
>>> two codebases. I managed to remove a reasonable chunk of code.
>>>
>>>   lib/param/loadparm.c         | 239 ++++++++++++++++++++++++++----------
>>>   lib/param/loadparm.h         |   1 -
>>>   lib/param/s3_param.h         |   5 -
>>>   source3/param/loadparm.c     | 414
>>> +++++++++-----------------------------------------------------
>>>   source3/param/loadparm_ctx.c |   5 -
>>>   5 files changed, 235 insertions(+), 429 deletions(-)
>>>
>>>
>>> Changes from now on should hopefully be quite a bit easier than it
>>> was back when I started.
>>>
>>> In particular, performing a 'do_parameter' is now equivalent and
>>> setting defaults could be done from the same code.
>>>
>>> A review would be appreciated.
>> I started to look at the patches, and while
>> the general train of thought is obvious and
>> the patches are clear, and most of them look
>> good. But I would like to verify
>> in a few places that the replaced calls are
>> really equivalend.
>>
>>
>> One trivial comment right away:
>>
>>> >From 8d0a409e41728dd26b580d4bc722e4ebbe02efc4 Mon Sep 17 00:00:00 2001
>>> From: Garming Sam <garming at catalyst.net.nz>
>>> Date: Fri, 21 Mar 2014 09:36:04 +1300
>>> Subject: [PATCH 09/28] lib/param: fix comment in lpcfg_service_ok
>>>
>>> Change-Id: I111836be6d556e0e63c1e4129216f01c74e5e0d5
>>> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
>>> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
>>> ---
>>>   lib/param/loadparm.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
>>> index 5aaef13..71ee4ad 100644
>>> --- a/lib/param/loadparm.c
>>> +++ b/lib/param/loadparm.c
>>> @@ -964,7 +964,7 @@ bool lpcfg_service_ok(struct loadparm_service *service)
>>>   		service->bAvailable = false;
>>>   	}
>>>   
>>> -	/* If a service is flagged unavailable, log the fact at level 0. */
>>> +	/* If a service is flagged unavailable, log the fact at level 1. */
>>>   	if (!service->bAvailable)
>>>   		DEBUG(1, ("NOTE: Service %s is flagged unavailable.\n",
>>>   			  service->szService));
>>> -- 
>>> 1.8.3.2
>> I'd rather remove that comment altogehter: It is redundant.
>> Alternative patch attached. ;-)
>>
>> Cheers - Michael
>>
>>
>>  From 039412b071ec12492ec1163b88ae45c6e8220335 Mon Sep 17 00:00:00 2001
>> From: Michael Adam <obnox at samba.org>
>> Date: Sat, 26 Jul 2014 02:17:07 +0200
>> Subject: [PATCH] lib/param: remove a redundant (and wrong) comment from
>>   lpcfg_service_ok()
>>
>> Signed-off-by: Michael Adam <obnox at samba.org>
>> ---
>>   lib/param/loadparm.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
>> index b58a058..cfee591 100644
>> --- a/lib/param/loadparm.c
>> +++ b/lib/param/loadparm.c
>> @@ -964,7 +964,6 @@ static bool lpcfg_service_ok(struct loadparm_service *service)
>>   			service->browseable = false;
>>   	}
>>   
>> -	/* If a service is flagged unavailable, log the fact at level 0. */
>>   	if (!service->bAvailable)
>>   		DEBUG(1, ("NOTE: Service %s is flagged unavailable.\n",
>>   			  service->szService));
>> -- 
>> 1.9.1
>>
>
>



More information about the samba-technical mailing list