[PATCH] Merging loadparm further

Michael Adam obnox at samba.org
Fri Jul 25 18:20:02 MDT 2014


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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lib-param-remove-a-redundant-and-wrong-comment-from-.patch
Type: text/x-diff
Size: 808 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140726/64807f14/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140726/64807f14/attachment.pgp>


More information about the samba-technical mailing list