On 08/03/2016 12:34 PM, Michal Židek wrote:
Two nitpicks, see inline.

On 07/22/2016 02:34 PM, Petr Cech wrote:

+static errno_t add_to_missing_attrs (TALLOC_CTX * mem_ctx,
+                                     struct sysdb_attrs *attrs,
+                                     const char *ext_key,
+                                     char ***_missing)
                                       ^
Coding style. Remove the space between function name and "(".
Do not forget to align the parameters after that.

Addressed.

+{
+    bool is_present = false;
+    size_t size = 0;
+    size_t ret;
+
+    for (int i = 0; i < attrs->num; i++) {
+        if (strcmp(ext_key, attrs->a[i].name) == 0) {
+            is_present = true;
+        }
+        size++;
+    }
+
+    if (is_present == false) {
+        ret = add_string_to_list(attrs, ext_key, _missing);
+        if (ret != EOK) {
+            goto fail;
+        }
+    }
+
+    ret = EOK;
+
+fail:

Please change the label name to "done". According to
our new coding style, the code that follows label "fail"
is only executed when failure occurs. I know we do not
follow this everywhere,  but I would like to be consistent
in new code.

Addressed.

+    return ret;
+}
+

Other than that it looks good to me.

Also it would be good to add a CI tests for this. I do
not want to postpone this patch before release, so you can
do it later as part of this ticket:
https://fedorahosted.org/sssd/ticket/3119

So either send a patch with CI test now or
assign the above ticket to yourself and do it
when there is more time.

Michal

Hello,

there is fixed patch attached.

I am working on CI test. I hope it will be ready soon.

Regards.

--
Petr^4 Čech
>From f84d4d2fcc7c1f2cb404a02aeea7abd12c14dd61 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Fri, 22 Jul 2016 14:28:54 +0200
Subject: [PATCH] LDAP: Fixing of removing netgroup from cache

There were problem with local key which wasn't properly removed.
This patch fixes it.

Resolves:
https://fedorahosted.org/sssd/ticket/2841
---
 src/providers/ldap/sdap_async_netgroups.c | 40 +++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c
index df233d956df70cfcb5f68bd2afc9e2a23c50c3bb..cf7d7b12361f8cc578b891961c0c5566442f1b4e 100644
--- a/src/providers/ldap/sdap_async_netgroups.c
+++ b/src/providers/ldap/sdap_async_netgroups.c
@@ -38,6 +38,35 @@ bool is_dn(const char *str)
     return (ret == LDAP_SUCCESS ? true : false);
 }
 
+static errno_t add_to_missing_attrs(TALLOC_CTX * mem_ctx,
+                                    struct sysdb_attrs *attrs,
+                                    const char *ext_key,
+                                    char ***_missing)
+{
+    bool is_present = false;
+    size_t size = 0;
+    size_t ret;
+
+    for (int i = 0; i < attrs->num; i++) {
+        if (strcmp(ext_key, attrs->a[i].name) == 0) {
+            is_present = true;
+        }
+        size++;
+    }
+
+    if (is_present == false) {
+        ret = add_string_to_list(attrs, ext_key, _missing);
+        if (ret != EOK) {
+            goto done;
+        }
+    }
+
+    ret = EOK;
+
+done:
+    return ret;
+}
+
 static errno_t sdap_save_netgroup(TALLOC_CTX *memctx,
                                   struct sss_domain_info *dom,
                                   struct sdap_options *opts,
@@ -138,6 +167,17 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx,
         goto fail;
     }
 
+    /* Prepare SYSDB_NETGROUP_MEMBER removing
+     * if not present in netgroup_attrs
+     */
+    ret = add_to_missing_attrs(attrs, netgroup_attrs, SYSDB_NETGROUP_MEMBER,
+                               &missing);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add [%s] to missing attributes\n",
+              SYSDB_NETGROUP_MEMBER);
+        goto fail;
+    }
+
     ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing,
                              dom->netgroup_timeout, now);
     if (ret) goto fail;
-- 
2.7.4

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

Reply via email to