[PATCH] Two small cleanups
Jeremy Allison
jra at samba.org
Thu Mar 16 17:11:37 UTC 2017
On Thu, Mar 16, 2017 at 06:04:26PM +0100, Ralph Böhme via samba-technical wrote:
> On Thu, Mar 16, 2017 at 04:29:40PM +0100, Volker Lendecke wrote:
> > On Thu, Mar 16, 2017 at 10:56:17AM +0100, Ralph Böhme via samba-technical wrote:
> > > Hi!
> > >
> > > Attached are two small cleanups for winbindd.
> > >
> > > Please review & push if happy. Thanks!
> >
> > Attached with my Review. While there -- I've added a patch on top to
> > squash. When touching code anyway I would appreciated to change it to
> > <80 chars per line.
>
> I stopped after the two patches fixing the two things that annoyed me most and
> then forcing me to stop. Otherwise I would have done the long line fix as well
> plus another two fixes, ending with a patchset like the one attached. :)
>
> Please review&push if happy. Thanks!
Bit late. I already pushed the original + squashed patches
as I was already doing an autobuild and just swept up
everything available on the list at the time (autobuilds
are *slow* these days :-).
Will apply this on top if it goes through, replace with
this if not !
Jeremy.
> From 4d26ccf227316951a2cba58496483d5fd5b5b82b Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 16 Mar 2017 09:32:55 +0100
> Subject: [PATCH 1/5] winbindd: use NULL for pointer check in get_cache()
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> Reviewed-by: Volker Lendecke <vl at samba.org>
> ---
> source3/winbindd/winbindd_cache.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
> index bafeb9b..f0c19fb 100644
> --- a/source3/winbindd/winbindd_cache.c
> +++ b/source3/winbindd/winbindd_cache.c
> @@ -162,7 +162,7 @@ static struct winbind_cache *get_cache(struct winbindd_domain *domain)
> --jerry
> */
>
> - if (!domain->backend) {
> + if (domain->backend == NULL) {
> #ifdef HAVE_ADS
> struct winbindd_domain *our_domain = domain;
>
> --
> 2.9.3
>
>
> From 7f1cc9f49437afcc1625f0b2caad7d80f89f7617 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 16 Mar 2017 10:36:14 +0100
> Subject: [PATCH 2/5] winbindd: untangle reconnect_methods vs
> reconnect_ads_methods
>
> No change in behaviour. The previous logic just seemed a bit clumsy
> because of the ifdefs.
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> Reviewed-by: Volker Lendecke <vl at samba.org>
> ---
> source3/winbindd/winbindd_cache.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
> index f0c19fb..0ef7c16 100644
> --- a/source3/winbindd/winbindd_cache.c
> +++ b/source3/winbindd/winbindd_cache.c
> @@ -162,8 +162,8 @@ static struct winbind_cache *get_cache(struct winbindd_domain *domain)
> --jerry
> */
>
> - if (domain->backend == NULL) {
> #ifdef HAVE_ADS
> + if (domain->backend == NULL) {
> struct winbindd_domain *our_domain = domain;
>
> /* find our domain first so we can figure out if we
> @@ -177,13 +177,13 @@ static struct winbind_cache *get_cache(struct winbindd_domain *domain)
> && !lp_winbind_rpc_only()) {
> DEBUG(5,("get_cache: Setting ADS methods for domain %s\n", domain->name));
> domain->backend = &reconnect_ads_methods;
> - } else {
> -#endif /* HAVE_ADS */
> - DEBUG(5,("get_cache: Setting MS-RPC methods for domain %s\n", domain->name));
> - domain->backend = &reconnect_methods;
> -#ifdef HAVE_ADS
> }
> + }
> #endif /* HAVE_ADS */
> +
> + if (domain->backend == NULL) {
> + DEBUG(5,("get_cache: Setting MS-RPC methods for domain %s\n", domain->name));
> + domain->backend = &reconnect_methods;
> }
>
> if (ret)
> --
> 2.9.3
>
>
> From 33c3070927e620e9bfe5262789b389565bf37d8c Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 16 Mar 2017 17:45:36 +0100
> Subject: [PATCH 3/5] winbindd: fix long lines in get_cache()
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source3/winbindd/winbindd_cache.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
> index 0ef7c16..cad2843 100644
> --- a/source3/winbindd/winbindd_cache.c
> +++ b/source3/winbindd/winbindd_cache.c
> @@ -175,14 +175,15 @@ static struct winbind_cache *get_cache(struct winbindd_domain *domain)
> if ((our_domain->active_directory || IS_DC)
> && domain->active_directory
> && !lp_winbind_rpc_only()) {
> - DEBUG(5,("get_cache: Setting ADS methods for domain %s\n", domain->name));
> + DBG_INFO("Setting ADS methods for domain %s\n",
> + domain->name);
> domain->backend = &reconnect_ads_methods;
> }
> }
> #endif /* HAVE_ADS */
>
> if (domain->backend == NULL) {
> - DEBUG(5,("get_cache: Setting MS-RPC methods for domain %s\n", domain->name));
> + DBG_INFO("Setting MS-RPC methods for domain %s\n", domain->name);
> domain->backend = &reconnect_methods;
> }
>
> --
> 2.9.3
>
>
> From 2a1f104422dc4feb3674dcd4fecb2cc1671c0670 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 16 Mar 2017 17:51:29 +0100
> Subject: [PATCH 4/5] winbindd: README.Coding fixes for get_cache()
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source3/winbindd/winbindd_cache.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
> index cad2843..f730f75 100644
> --- a/source3/winbindd/winbindd_cache.c
> +++ b/source3/winbindd/winbindd_cache.c
> @@ -135,11 +135,12 @@ static struct winbind_cache *get_cache(struct winbindd_domain *domain)
> }
>
> if (strequal(domain->name, get_global_sam_name()) &&
> - sid_check_is_our_sam(&domain->sid)) {
> + sid_check_is_our_sam(&domain->sid))
> + {
> domain->backend = &sam_passdb_methods;
> }
>
> - if ( !domain->initialized ) {
> + if (!domain->initialized) {
> /* We do not need a connection to an RW DC for cache operation */
> init_dc_connection(domain, false);
> }
> @@ -169,12 +170,14 @@ static struct winbind_cache *get_cache(struct winbindd_domain *domain)
> /* find our domain first so we can figure out if we
> are joined to a kerberized domain */
>
> - if ( !domain->primary )
> + if (!domain->primary) {
> our_domain = find_our_domain();
> + }
>
> if ((our_domain->active_directory || IS_DC)
> && domain->active_directory
> - && !lp_winbind_rpc_only()) {
> + && !lp_winbind_rpc_only())
> + {
> DBG_INFO("Setting ADS methods for domain %s\n",
> domain->name);
> domain->backend = &reconnect_ads_methods;
> @@ -187,8 +190,9 @@ static struct winbind_cache *get_cache(struct winbindd_domain *domain)
> domain->backend = &reconnect_methods;
> }
>
> - if (ret)
> + if (ret != NULL) {
> return ret;
> + }
>
> ret = SMB_XMALLOC_P(struct winbind_cache);
> ZERO_STRUCTP(ret);
> --
> 2.9.3
>
>
> From b0ed5e1c4febf5637db325c1540632fc0c5cc406 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 16 Mar 2017 17:52:50 +0100
> Subject: [PATCH 5/5] winbindd: remove trailing spaces in get_cache()
>
> Trailing spaces are annoyingly highlighted red in my emacs setup so I'd
> like to get rid of them. :)
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source3/winbindd/winbindd_cache.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
> index f730f75..98c69f8 100644
> --- a/source3/winbindd/winbindd_cache.c
> +++ b/source3/winbindd/winbindd_cache.c
> @@ -145,17 +145,17 @@ static struct winbind_cache *get_cache(struct winbindd_domain *domain)
> init_dc_connection(domain, false);
> }
>
> - /*
> + /*
> OK. Listen up because I'm only going to say this once.
> We have the following scenarios to consider
> (a) trusted AD domains on a Samba DC,
> (b) trusted AD domains and we are joined to a non-kerberos domain
> (c) trusted AD domains and we are joined to a kerberos (AD) domain
>
> - For (a) we can always contact the trusted domain using krb5
> + For (a) we can always contact the trusted domain using krb5
> since we have the domain trust account password
>
> - For (b) we can only use RPC since we have no way of
> + For (b) we can only use RPC since we have no way of
> getting a krb5 ticket in our own domain
>
> For (c) we can always use krb5 since we have a kerberos trust
> @@ -167,7 +167,7 @@ static struct winbind_cache *get_cache(struct winbindd_domain *domain)
> if (domain->backend == NULL) {
> struct winbindd_domain *our_domain = domain;
>
> - /* find our domain first so we can figure out if we
> + /* find our domain first so we can figure out if we
> are joined to a kerberized domain */
>
> if (!domain->primary) {
> --
> 2.9.3
>
More information about the samba-technical
mailing list