[PATCHES] GPO: fail if there is problem with storing gpo into sysdb

[Lukas Slebodnik]

ehlo,

some variable could be used unitialized.

Simple patches are attached.

LS

[Jakub Hrozek]

On Sat, Oct 11, 2014 at 11:49:54PM +0200, Lukas Slebodnik wrote:
> ehlo,
>
> some variable could be used unitialized.
>
> Simple patches are attached.
>
> LS
> From f2a7faefba6e6191f324c4c33609ccb7570835e5 Mon Sep 17 00:00:00 2001
> From: Lukas Slebodnik 
> Date: Sat, 11 Oct 2014 12:55:05 +0200
> Subject: [PATCH 7/9] GPO: fail if there is problem with storing gpo into sysdb
>
> src/providers/ad/ad_gpo.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
> index 9f52aedf8d10d6a870846f311b5347896fba31bd..6fdcbe3fe65b31d56123ddea95a5e46282cfb599 100644
> --- a/src/providers/ad/ad_gpo.c
> +++ b/src/providers/ad/ad_gpo.c
> @@ -1147,6 +1147,11 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
>                  ret = sysdb_gpo_store_gpo_result_setting(domain,
>                                                           allow_key,
>                                                           allow_value);
>            DEBUG(SSSDBG_CRIT_FAILURE,
>                  "sysdb_gpo_store_gpo_result_setting failed for key:'%s' "
>                  "value:'%s' [%d][%s]\n", allow_key, allow_value,
>                  ret, sss_strerror(ret));
>            goto done;

I think the debug and goto should be wrapped in an if statement here.
>          }
>      }
>
> @@ -1166,6 +1171,11 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
>                  ret = sysdb_gpo_store_gpo_result_setting(domain,
>                                                           deny_key,
>                                                           deny_value);
>            DEBUG(SSSDBG_CRIT_FAILURE,
>                  "sysdb_gpo_store_gpo_result_setting failed for key:'%s' "
>                  "value:'%s' [%d][%s]\n", deny_key, deny_value,
>                  ret, sss_strerror(ret));
>            goto done;

Same here
>          }
>      }
>  }
>
> -- 
> 2.1.0
> From f7b9445fd64524e3217ebff99d0b80499732991d Mon Sep 17 00:00:00 2001
> From: Lukas Slebodnik 
> Date: Sat, 11 Oct 2014 13:10:18 +0200
> Subject: [PATCH 8/9] GPO: Fail if we cannot retrieve gpo from cache.

ACK
> From d51d63d81e22bf2e3cc970e96505611f72747a68 Mon Sep 17 00:00:00 2001
> From: Lukas Slebodnik 
> Date: Sat, 11 Oct 2014 21:38:32 +0200
> Subject: [PATCH 9/9] GPO: Do not use output argument if function failed

ACK

[Lukas Slebodnik]

On (12/10/14 17:21), Jakub Hrozek wrote:
> On Sat, Oct 11, 2014 at 11:49:54PM +0200, Lukas Slebodnik wrote:
> > ehlo,
> >
> > some variable could be used unitialized.
> >
> > Simple patches are attached.
> >
> > LS
> > From f2a7faefba6e6191f324c4c33609ccb7570835e5 Mon Sep 17 00:00:00 2001
> > From: Lukas Slebodnik 
> > Date: Sat, 11 Oct 2014 12:55:05 +0200
> > Subject: [PATCH 7/9] GPO: fail if there is problem with storing gpo into sysdb
> >
> > src/providers/ad/ad_gpo.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
> > index 9f52aedf8d10d6a870846f311b5347896fba31bd..6fdcbe3fe65b31d56123ddea95a5e46282cfb599 100644
> > --- a/src/providers/ad/ad_gpo.c
> > +++ b/src/providers/ad/ad_gpo.c
> > @@ -1147,6 +1147,11 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
> >                  ret = sysdb_gpo_store_gpo_result_setting(domain,
> >                                                           allow_key,
> >                                                           allow_value);
> >            DEBUG(SSSDBG_CRIT_FAILURE,
> >                  "sysdb_gpo_store_gpo_result_setting failed for key:'%s' "
> >                  "value:'%s' [%d][%s]\n", allow_key, allow_value,
> >                  ret, sss_strerror(ret));
> >            goto done;
>
> I think the debug and goto should be wrapped in an if statement here.
> >          }
> >      }
> >
> > @@ -1166,6 +1171,11 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
> >                  ret = sysdb_gpo_store_gpo_result_setting(domain,
> >                                                           deny_key,
> >                                                           deny_value);
> >            DEBUG(SSSDBG_CRIT_FAILURE,
> >                  "sysdb_gpo_store_gpo_result_setting failed for key:'%s' "
> >                  "value:'%s' [%d][%s]\n", deny_key, deny_value,
> >                  ret, sss_strerror(ret));
> >            goto done;
>
> Same here

Copy paste problem.
Fixed version is attached.

LS

[Jakub Hrozek]

On Mon, Oct 13, 2014 at 09:58:56AM +0200, Lukas Slebodnik wrote:
> On (12/10/14 17:21), Jakub Hrozek wrote:
> > On Sat, Oct 11, 2014 at 11:49:54PM +0200, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > some variable could be used unitialized.
> > >
> > > Simple patches are attached.
> > >
> > > LS
> > > From f2a7faefba6e6191f324c4c33609ccb7570835e5 Mon Sep 17 00:00:00 2001
> > > From: Lukas Slebodnik 
> > > Date: Sat, 11 Oct 2014 12:55:05 +0200
> > > Subject: [PATCH 7/9] GPO: fail if there is problem with storing gpo into sysdb
> > >
> > > src/providers/ad/ad_gpo.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
> > > index 9f52aedf8d10d6a870846f311b5347896fba31bd..6fdcbe3fe65b31d56123ddea95a5e46282cfb599 100644
> > > --- a/src/providers/ad/ad_gpo.c
> > > +++ b/src/providers/ad/ad_gpo.c
> > > @@ -1147,6 +1147,11 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
> > >                  ret = sysdb_gpo_store_gpo_result_setting(domain,
> > >                                                           allow_key,
> > >                                                           allow_value);
> > >            DEBUG(SSSDBG_CRIT_FAILURE,
> > >                  "sysdb_gpo_store_gpo_result_setting failed for key:'%s' "
> > >                  "value:'%s' [%d][%s]\n", allow_key, allow_value,
> > >                  ret, sss_strerror(ret));
> > >            goto done;
> >
> > I think the debug and goto should be wrapped in an if statement here.
> > >          }
> > >      }
> > >
> > > @@ -1166,6 +1171,11 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
> > >                  ret = sysdb_gpo_store_gpo_result_setting(domain,
> > >                                                           deny_key,
> > >                                                           deny_value);
> > >            DEBUG(SSSDBG_CRIT_FAILURE,
> > >                  "sysdb_gpo_store_gpo_result_setting failed for key:'%s' "
> > >                  "value:'%s' [%d][%s]\n", deny_key, deny_value,
> > >                  ret, sss_strerror(ret));
> > >            goto done;
> >
> > Same here
>
> Copy paste problem.
> Fixed version is attached.
>
> LS

ACK

[Jakub Hrozek]

On Mon, Oct 13, 2014 at 04:11:48PM +0200, Jakub Hrozek wrote:
> > Copy paste problem.
> > Fixed version is attached.
> >
> > LS
>
> ACK

Pushed master:
* b083fec6c175989e0d32b83d8cc857a6b54d26a2
* dd09bbb2578bd9f82afd7fc4d4fcc3a5659731d3
* 76ba5d2b8d95ec444c124dd9a44275ce685fb648