[SSSD] [PATCH] sdap: Fix ldap_rfc_2307_fallback_to_local_users

2016-07-13 Thread Michal Židek 

Hi,

see the attached simple patch for ticket:
https://fedorahosted.org/sssd/ticket/3045

The patch is missing a CI test. I will add
one (hopefully later tomorrow) after I take
a look at one bugzilla which has currently
higher priority.

If someone writes a test for this until then,
I will gladly review it :)

The reproducer is simple:
1. have ldap with RFC2307 schema with group
   that contains user from /etc/passwd (for example
   local_user)
2. run 'id local_user'
3. the ldap group should be among the displayed groups


Michal
>From c324ca57d5bed4ad2a290d819ad84349d45cc669 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Wed, 13 Jul 2016 20:02:47 +0200
Subject: [PATCH] sdap: Fix ldap_rfc_2307_fallback_to_local_users

Fixes:
https://fedorahosted.org/sssd/ticket/3045

We wrongly tried to store empty
user attributes instead of the
local user attributes with
ldap_rfc_2307_fallback_to_local_users
set to true. This gave us bad
initgroups results and caused
segfaults.
---
 src/providers/ldap/sdap_async_initgroups.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index d14563c..17593f0 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -2893,6 +2893,9 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
 (dp_opt_get_bool(state->opts->basic,
  SDAP_RFC2307_FALLBACK_TO_LOCAL_USERS) == true)) {
 ret = sdap_fallback_local_user(state, state->shortname, -1, _attrs);
+if (ret == EOK) {
+state->orig_user = usr_attrs[0];
+}
 } else {
 ret = ENOENT;
 }
-- 
2.5.0

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [PATCH] intg: test nested membership

2016-07-13 Thread Lukas Slebodnik 
ehlo,

attched patch is an integration test for regression #3093.
I prepared a test and I let someone else to fix it :-)

I will try to find more bugs in downstream tests.

BTW we might move test from test_ts_cache somewhere else.
But it was the fastest way how to write a test without enumeration.

LS
>From 635281c970351414d0624b3e6efc1762b1188a3c Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Wed, 13 Jul 2016 17:26:03 +0200
Subject: [PATCH] intg: test nested membership

Integration fes for #3093
---
 src/tests/intg/test_ts_cache.py | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/src/tests/intg/test_ts_cache.py b/src/tests/intg/test_ts_cache.py
index 
0ba84b45f57ca246d28bfa0478a2c0db7e7a99e0..57c6264a6a6230bd6c56ce2b337bee9209f98572
 100644
--- a/src/tests/intg/test_ts_cache.py
+++ b/src/tests/intg/test_ts_cache.py
@@ -130,6 +130,9 @@ def load_data_to_ldap(request, ldap_conn, schema):
 
 if schema == SCHEMA_RFC2307_BIS:
 ent_list.add_group_bis("group1", 2001, ("user1", "user11", "user21"))
+ent_list.add_group_bis("group10", 2010, ["user1"])
+ent_list.add_group_bis("group11", 2011, ["user1"])
+ent_list.add_group_bis("group20", 2020, [], ["group10", "group11"])
 elif schema == SCHEMA_RFC2307:
 ent_list.add_group("group1", 2001, ("user1", "user11", "user21"))
 create_ldap_fixture(request, ldap_conn, ent_list)
@@ -605,3 +608,26 @@ def test_user_2307bis_delete_user(ldap_conn,
 assert sysdb_attrs.get("originalModifyTimestamp") is None
 assert ts_attrs.get("dataExpireTimestamp") is None
 assert ts_attrs.get("originalModifyTimestamp") is None
+
+
+def test_user_2307bis_nested_groups(ldap_conn,
+ldb_examine,
+setup_rfc2307bis):
+"""
+Test nested groups
+"""
+primary_gid = 2001
+# group1, group10, group11, group20
+expected_gids = [ 2001, 2010, 2011, 2020 ]
+
+ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001,
+gid=primary_gid))
+
+(res, errno, gids) = sssd_id.call_sssd_initgroups("user1", primary_gid)
+assert res == sssd_id.NssReturnCode.SUCCESS
+
+assert sorted(gids) == sorted(expected_gids), \
+"result: %s\n expected %s" % (
+", ".join(["%s" % s for s in sorted(gids)]),
+", ".join(["%s" % s for s in sorted(expected_gids)])
+)
-- 
2.7.4

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [PATCH] sssctl: use internal API to remove files

2016-07-13 Thread Pavel Březina 
 From 0aa39a46b707212e6487b6b537238e31bf7da1b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 12:17:58 +0200
Subject: [PATCH 1/2] utils: add remove_subtree

Remove all entries in a directory but will
not remove the directory itself.
---
 src/tests/files-tests.c | 53 +
 src/tools/files.c   | 34 ---
 src/tools/tools_util.h  |  1 +
 3 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/src/tests/files-tests.c b/src/tests/files-tests.c
index 596069e2858e07953b3d48f6b8015ce66e2dd423..e96a60af1817b5f7a2e99d8b09ebc91c1a52667b 100644
--- a/src/tests/files-tests.c
+++ b/src/tests/files-tests.c
@@ -153,6 +153,58 @@ START_TEST(test_remove_tree)
 }
 END_TEST
 
+START_TEST(test_remove_subtree)
+{
+int ret;
+char origpath[PATH_MAX+1];
+
+errno = 0;
+fail_unless(getcwd(origpath, PATH_MAX) == origpath, "Cannot getcwd\n");
+fail_unless(errno == 0, "Cannot getcwd\n");
+
+DEBUG(SSSDBG_FUNC_DATA, "About to delete %s\n", dir_path);
+
+/* create a file */
+ret = chdir(dir_path);
+fail_if(ret == -1, "Cannot chdir1\n");
+
+ret = create_simple_file("bar", "bar");
+fail_if(ret == -1, "Cannot create file1\n");
+
+/* create a subdir and file inside it */
+ret = mkdir("subdir", 0700);
+fail_if(ret == -1, "Cannot create subdir\n");
+
+ret = chdir("subdir");
+fail_if(ret == -1, "Cannot chdir\n");
+
+ret = create_simple_file("foo", "foo");
+fail_if(ret == -1, "Cannot create file\n");
+
+/* create another subdir, empty this time */
+ret = mkdir("subdir2", 0700);
+fail_if(ret == -1, "Cannot create subdir\n");
+
+ret = chdir(origpath);
+fail_if(ret == -1, "Cannot chdir2\n");
+
+/* go back */
+ret = chdir(origpath);
+fail_if(ret == -1, "Cannot chdir\n");
+
+/* and finally wipe it out.. */
+ret = remove_subtree(dir_path);
+fail_unless(ret == EOK, "remove_subtree failed\n");
+
+/* check if really gone */
+ret = access(dir_path, F_OK);
+fail_unless(ret == 0, "directory was deleted\n");
+
+ret = rmdir(dir_path);
+fail_unless(ret == 0, "unable to delete root directory\n");
+}
+END_TEST
+
 START_TEST(test_simple_copy)
 {
 int ret;
@@ -337,6 +389,7 @@ static Suite *files_suite(void)
   teardown_files_test);
 
 tcase_add_test(tc_files, test_remove_tree);
+tcase_add_test(tc_files, test_remove_subtree);
 tcase_add_test(tc_files, test_simple_copy);
 tcase_add_test(tc_files, test_copy_file);
 tcase_add_test(tc_files, test_copy_symlink);
diff --git a/src/tools/files.c b/src/tools/files.c
index 5364f5c0dd53aad71452e18b8d7f1f04532132a4..182048be4dca9b6c5255695409fe9e52b730c744 100644
--- a/src/tools/files.c
+++ b/src/tools/files.c
@@ -137,7 +137,8 @@ static int sss_futime_set(int fd, const struct stat *statp)
 static int remove_tree_with_ctx(TALLOC_CTX *mem_ctx,
 int parent_fd,
 const char *dir_name,
-dev_t parent_dev);
+dev_t parent_dev,
+bool subtree);
 
 int remove_tree(const char *root)
 {
@@ -149,7 +150,22 @@ int remove_tree(const char *root)
 return ENOMEM;
 }
 
-ret = remove_tree_with_ctx(tmp_ctx, AT_FDCWD, root, 0);
+ret = remove_tree_with_ctx(tmp_ctx, AT_FDCWD, root, 0, false);
+talloc_free(tmp_ctx);
+return ret;
+}
+
+int remove_subtree(const char *root)
+{
+TALLOC_CTX *tmp_ctx = NULL;
+int ret;
+
+tmp_ctx = talloc_new(NULL);
+if (!tmp_ctx) {
+return ENOMEM;
+}
+
+ret = remove_tree_with_ctx(tmp_ctx, AT_FDCWD, root, 0, true);
 talloc_free(tmp_ctx);
 return ret;
 }
@@ -162,7 +178,8 @@ int remove_tree(const char *root)
 static int remove_tree_with_ctx(TALLOC_CTX *mem_ctx,
 int parent_fd,
 const char *dir_name,
-dev_t parent_dev)
+dev_t parent_dev,
+bool subtree)
 {
 struct dirent *result;
 struct stat statres;
@@ -213,7 +230,8 @@ static int remove_tree_with_ctx(TALLOC_CTX *mem_ctx,
 goto fail;
 }
 
-ret = remove_tree_with_ctx(mem_ctx, dir_fd, result->d_name, statres.st_dev);
+ret = remove_tree_with_ctx(mem_ctx, dir_fd, result->d_name,
+   statres.st_dev, false);
 if (ret != EOK) {
 DEBUG(SSSDBG_CRIT_FAILURE,
   "Removing subdirectory failed: [%d][%s]\n",
@@ -238,9 +256,11 @@ static int remove_tree_with_ctx(TALLOC_CTX *mem_ctx,
 goto fail;
 }
 
-ret = unlinkat(parent_fd, dir_name, AT_REMOVEDIR);
-if (ret == -1) {
-ret = errno;
+if (!subtree) {
+ret =

[SSSD] Re: [PATCHES] sssctl: print active server and server list

2016-07-13 Thread Jakub Hrozek 
On Thu, Jun 30, 2016 at 02:10:47PM +0200, Pavel Březina wrote:
> Failover patches for the sssctl tool. The output looks like:
> 
> [master.ipa.pb: ~]$ sudo sssctl domain-status ad.pb
> Online status: Online
> 
> Active servers:
> AD Global Catalog: root-dc.ad.pb
> AD Domain Controller: root-dc.ad.pb
> IPA: master.ipa.pb
> 
> Discovered AD Global Catalog servers:
> - root-dc.ad.pb
> - invalid.ad.pb
> - root-dc.ad.pb
> 
> Discovered AD Domain Controller servers:
> - root-dc.ad.pb
> 
> Discovered IPA servers:
> - master.ipa.pb
> 
> This is the best output format I could thing of so far, but I'm opened for
> suggestions.

I'm sorry for the late review. I started by sending the patches to CI
which found some failures:
http://sssd-ci.duckdns.org/logs/job/49/69/summary.html
and also Coverity complains:
Error: CHECKED_RETURN (CWE-252):
sssd-1.14.1/src/lib/sifp/sss_sifp_dbus.c:51: check_return: Calling 
"dbus_message_append_args_valist" without checking return value (as is done 
elsewhere 4 out of 5 times).
sssd-1.14.1/src/responder/common/data_provider/rdp_message.c:59: 
example_assign: Example 1: Assigning: "bret" = return value from 
"dbus_message_append_args_valist(msg, first_arg_type, va)".
sssd-1.14.1/src/responder/common/data_provider/rdp_message.c:60: 
example_checked: Example 1 (cont.): "bret" has its value checked in "bret".
sssd-1.14.1/src/sbus/sssd_dbus_request.c:163: example_assign: Example 2: 
Assigning: "dbret" = return value from "dbus_message_append_args_valist(reply, 
first_arg_type, va)".
sssd-1.14.1/src/sbus/sssd_dbus_request.c:166: example_checked: Example 2 
(cont.): "dbret" has its value checked in "dbret".
sssd-1.14.1/src/sbus/sssd_dbus_utils.c:141: example_assign: Example 3: 
Assigning: "bret" = return value from "dbus_message_append_args_valist(msg, 
first_arg_type, va)".
sssd-1.14.1/src/sbus/sssd_dbus_utils.c:142: example_checked: Example 3 (cont.): 
"bret" has its value checked in "bret".
sssd-1.14.1/src/tools/sssctl/sssctl_sifp.c:142: example_assign: Example 4: 
Assigning: "bret" = return value from "dbus_message_append_args_valist(msg, 
first_arg_type, va)".
sssd-1.14.1/src/tools/sssctl/sssctl_sifp.c:143: example_checked: Example 4 
(cont.): "bret" has its value checked in "bret".
#   49|   
#   50|  if (first_arg_type != DBUS_TYPE_INVALID) {
#   51|->dbus_message_append_args_valist(msg, first_arg_type, ap);
#   52|  }
#   53|   

Error: UNINIT (CWE-457):
sssd-1.14.1/src/providers/data_provider/dp_iface_failover.c:182: var_decl: 
Declaring variable "ad_found" without initializer.
sssd-1.14.1/src/providers/data_provider/dp_iface_failover.c:236: uninit_use: 
Using uninitialized value "ad_found".
#  234|   
#  235|   /* Fill the list. */
#  236|-> if ((ad_found && ipa_found) || (!ad_found && !ipa_found)) {
#  237|   /* If AD and IPA was found it is some complicated 
configuration,
#  238|* we return everything. Otherwise it's LDAP. */

Error: UNINIT (CWE-457):
sssd-1.14.1/src/providers/data_provider/dp_iface_failover.c:183: var_decl: 
Declaring variable "ipa_found" without initializer.
sssd-1.14.1/src/providers/data_provider/dp_iface_failover.c:236: uninit_use: 
Using uninitialized value "ipa_found".
#  234|   
#  235|   /* Fill the list. */
#  236|-> if ((ad_found && ipa_found) || (!ad_found && !ipa_found)) {
#  237|   /* If AD and IPA was found it is some complicated 
configuration,
#  238|* we return everything. Otherwise it's LDAP. */

Error: VARARGS (CWE-237):
sssd-1.14.1/src/tools/sssctl/sssctl_sifp.c:141: va_init: Initializing va_list 
"va".
sssd-1.14.1/src/tools/sssctl/sssctl_sifp.c:163: missing_va_end: va_end was not 
called for "va".
#  161|   done:
#  162|   dbus_message_unref(msg);
#  163|-> return error;
#  164|   }
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: move filter creation to separate function

2016-07-13 Thread Jakub Hrozek 
On Wed, Jul 13, 2016 at 10:44:18AM +0200, Fabiano Fidêncio wrote:
> Thanks a lot!
> Acked-by: Fabiano Fidêncio 
> 

* master:
 * 3c6e15e8aa38d9dfa02a7255fad56149bdfb35a6
 * aa691837a2fa2fe2e38a55d576644074e0f45bd8
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] dyndns: Add checks for NULL

2016-07-13 Thread Jakub Hrozek 
On Wed, Jul 13, 2016 at 10:16:22AM +0200, Pavel Březina wrote:
> On 07/12/2016 03:50 PM, Michal Židek wrote:
> > On 07/12/2016 01:36 PM, Michal Židek wrote:
> > > On 07/12/2016 01:15 PM, Pavel Březina wrote:
> > > > On 07/12/2016 12:34 PM, Michal Židek wrote:
> > > > >   state->ipa_ctx->dyndns_ctx->last_refresh = time(NULL);
> > > > 
> > > > LGTM but maybe we should place the check before this line?
> > > 
> > > Not sure... I only added checks for the line with strcmp
> > > (which is where it segfaulted). If I moved
> > > it up where you suggest, there would be the question why
> > > I do not check for example ipa_ctx or ipa_ctx->dyndns_ctx.
> > > 
> > > But if you prefer having it there I can do it, but will
> > > also add a comment that the checks are relevant for the
> > > strcmp line.
> > > 
> > > Michal
> > > 
> > 
> > Ok, I moved it as Pavel suggested and added a comment
> > that should make it clear why the checks are there
> > and also amended the commit message as Jakub suggested.
> > 
> > Michal
> 
> Ack.
> 
> I asked for that since in my opinion it is more natural to set the last
> refresh time only upon successful refresh. But I'm not that familiar with
> dyndns to have strong opinion on what is better, thus it was a quastion.

 * master: b5f61f8963300c9ba011436f234e9e10224aff6d
 * sssd-1-13: ae8c7c4010cb8fda478526771ef12d2bb8bfeffa 

(I hope it's OK I moved the ticket into 1.13.x)
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] views: allow override added for non-default views at runtime

2016-07-13 Thread Jakub Hrozek 
On Wed, Jul 13, 2016 at 10:13:05AM +0200, Pavel Březina wrote:
> On 07/12/2016 02:46 PM, Jakub Hrozek wrote:
> > On Tue, Jul 12, 2016 at 11:45:07AM +0200, Pavel Březina wrote:
> > > On 07/11/2016 03:16 PM, Sumit Bose wrote:
> > > > Hi,
> > > > 
> > > > this patch should solve
> > > > https://fedorahosted.org/sssd/ticket/3092, please see commit message for
> > > > details.
> > > > 
> > > > bye,
> > > > Sumit
> > > 
> > > Ack.
> > 
> > CI: http://sssd-ci.duckdns.org/logs/job/49/56/summary.html (RHEL-6
> > failure is in test_add_remove_membership_rfc2307_bis)
> > 
> > * master: 26a3d4f2ef35a088e4c5fc928290052c89a2ff43
> > 
> > I would like to push this patch also into the stable branch, unless
> > anyone disagrees?
> 
> Go ahead.

* sssd-1-13: a77707fee46d9ffd9159780b58a797228889ad7f
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: move filter creation to separate function

2016-07-13 Thread Fabiano Fidêncio 
On Wed, Jul 13, 2016 at 10:41 AM, Pavel Březina  wrote:
> On 07/12/2016 01:49 PM, Fabiano Fidêncio wrote:
>>
>> On Tue, Jul 12, 2016 at 1:37 PM, Pavel Březina 
>> wrote:
>>>
>>> On 07/12/2016 01:31 PM, Fabiano Fidêncio wrote:


 Replying with comments in-line:

   From a4a2eb297e6e56e6d0d05ab3810ab87eb320ebdb Mon Sep 17 00:00:00 2001
 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
 Date: Tue, 12 Jul 2016 12:59:48 +0200
 Subject: [PATCH] sssctl: move filter creation to separate function

 ---
src/tools/sssctl/sssctl_cache.c | 93
 -
1 file changed, 54 insertions(+), 39 deletions(-)

 diff --git a/src/tools/sssctl/sssctl_cache.c
 b/src/tools/sssctl/sssctl_cache.c
 index

 e23bb89db95217e66a441b7e4d6d32e668486cc8..3318c51243c2b39feabf798091bbd5f370c2e053
 100644
 --- a/src/tools/sssctl/sssctl_cache.c
 +++ b/src/tools/sssctl/sssctl_cache.c
 @@ -285,32 +285,21 @@ done:
return ret;
}

 -static errno_t sssctl_find_object(TALLOC_CTX *mem_ctx,
 -  struct sss_domain_info *domains,
 -  struct sss_domain_info *domain,
 -  sssctl_basedn_fn basedn_fn,
 -  enum cache_object obj_type,
 -  const char *attr_name,
 -  const char *attr_value,
 -  const char **attrs,
 -  struct sysdb_attrs **_entry,
 -  struct sss_domain_info **_dom)
 +static const char *sssctl_create_filter(TALLOC_CTX *mem_ctx,
 +struct sss_domain_info *dom,
 +enum cache_object obj_type,
 +const char *attr_name,
 +const char *attr_value)
{
 -TALLOC_CTX *tmp_ctx;
 -struct sss_domain_info *dom;
 -struct sysdb_attrs *entry;
 -struct ldb_dn *base_dn;
 -bool fqn_provided;
 -bool qualify_attr = false;
 -char *filter;
 -errno_t ret;
const char *class;
 +const char *filter;
char *filter_value;
 +bool qualify_attr = false;

 -if (strcmp(attr_name, SYSDB_NAME) == 0 &&
 -(obj_type == CACHED_USER ||
 - obj_type == CACHED_GROUP)) {
 -qualify_attr = true;
 +if (strcmp(attr_name, SYSDB_NAME) == 0) {
 +if (obj_type == CACHED_USER || obj_type == CACHED_GROUP) {
 +qualify_attr = true;
 +}

 This change seems unrelated to this patch.
 I understand someone can argue it improves readability, but then
 someone can argue the opposite ...
>>>
>>>
>>>
>>> And on which side are you? :-)
>>
>>
>> My personal preference: it's cleaner in the way you proposed.
>> But I really would prefer it coming in a follow up "cleaning up patch"
>> instead of sneaked into this one :-)
>>
>> And I noticed you didn't change it in the v2 :-)
>>
>>>
>>>

}

switch (obj_type) {
 @@ -326,9 +315,49 @@ static errno_t sssctl_find_object(TALLOC_CTX
 *mem_ctx,
default:
DEBUG(SSSDBG_FATAL_FAILURE,
  "sssctl doesn't handle this object type (type=%d)\n",
 obj_type);
 -return EINVAL;
 +return NULL;
}

 +if (qualify_attr) {
 +filter_value = sss_create_internal_fqname(NULL, attr_value,
 dom->name);
 +} else {
 +filter_value = talloc_strdup(NULL, attr_value);
 +}
 +if (filter_value == NULL) {
 +DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory!\n");
 +return NULL;
 +}
 +
 +filter = talloc_asprintf(mem_ctx, "(&(objectClass=%s)(%s=%s))",
 + class, attr_name, filter_value);
 +talloc_free(filter_value);
 +if (filter == NULL) {
 +DEBUG(SSSDBG_CRIT_FAILURE, "talloc_asprintf() failed\n");
 +return NULL;
 +}

 I wouldn't do this check/print this error message here, as a similar
 check is already done by the caller.

 +
 +return filter;
 +}
 +
 +static errno_t sssctl_find_object(TALLOC_CTX *mem_ctx,
 +  struct sss_domain_info *domains,
 +  struct sss_domain_info *domain,
 +  sssctl_basedn_fn basedn_fn,
 +  enum cache_object obj_type,
 +  const char *attr_name,
 +

[SSSD] Re: [PATCH] dyndns: Add checks for NULL

2016-07-13 Thread Pavel Březina 

On 07/12/2016 03:50 PM, Michal Židek wrote:

On 07/12/2016 01:36 PM, Michal Židek wrote:

On 07/12/2016 01:15 PM, Pavel Březina wrote:

On 07/12/2016 12:34 PM, Michal Židek wrote:

  state->ipa_ctx->dyndns_ctx->last_refresh = time(NULL);


LGTM but maybe we should place the check before this line?


Not sure... I only added checks for the line with strcmp
(which is where it segfaulted). If I moved
it up where you suggest, there would be the question why
I do not check for example ipa_ctx or ipa_ctx->dyndns_ctx.

But if you prefer having it there I can do it, but will
also add a comment that the checks are relevant for the
strcmp line.

Michal



Ok, I moved it as Pavel suggested and added a comment
that should make it clear why the checks are there
and also amended the commit message as Jakub suggested.

Michal


Ack.

I asked for that since in my opinion it is more natural to set the last 
refresh time only upon successful refresh. But I'm not that familiar 
with dyndns to have strong opinion on what is better, thus it was a 
quastion.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] views: allow override added for non-default views at runtime

2016-07-13 Thread Pavel Březina 

On 07/12/2016 02:46 PM, Jakub Hrozek wrote:

On Tue, Jul 12, 2016 at 11:45:07AM +0200, Pavel Březina wrote:

On 07/11/2016 03:16 PM, Sumit Bose wrote:

Hi,

this patch should solve
https://fedorahosted.org/sssd/ticket/3092, please see commit message for
details.

bye,
Sumit


Ack.


CI: http://sssd-ci.duckdns.org/logs/job/49/56/summary.html (RHEL-6
failure is in test_add_remove_membership_rfc2307_bis)

* master: 26a3d4f2ef35a088e4c5fc928290052c89a2ff43

I would like to push this patch also into the stable branch, unless
anyone disagrees?


Go ahead.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org