[PATCH] Two small cleanups

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] Two small cleanups

Samba - samba-technical mailing list
Hi!

Attached are two small cleanups for winbindd.

Please review & push if happy. Thanks!

Cheerio!
-slow
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Two small cleanups

Samba - samba-technical mailing list
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.

Volker

patch.txt (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Two small cleanups

Samba - samba-technical mailing list
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!

Cheerio!
-slow
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Two small cleanups

Samba - samba-technical mailing list
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 <[hidden email]>
> 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 <[hidden email]>
> Reviewed-by: Volker Lendecke <[hidden email]>
> ---
>  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 <[hidden email]>
> 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 <[hidden email]>
> Reviewed-by: Volker Lendecke <[hidden email]>
> ---
>  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 <[hidden email]>
> 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 <[hidden email]>
> ---
>  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 <[hidden email]>
> 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 <[hidden email]>
> ---
>  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 <[hidden email]>
> 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 <[hidden email]>
> ---
>  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
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Two small cleanups

Samba - samba-technical mailing list
On Thu, Mar 16, 2017 at 10:11:37AM -0700, Jeremy Allison via samba-technical wrote:

> 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 !

And it failed, so pushing this instead !

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Two small cleanups

Samba - samba-technical mailing list
On Thu, Mar 16, 2017 at 12:29:48PM -0700, Jeremy Allison wrote:

> On Thu, Mar 16, 2017 at 10:11:37AM -0700, Jeremy Allison via samba-technical wrote:
> > 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 !
>
> And it failed, so pushing this instead !

:) Thanks!

Cheerio!
-slow

Loading...