Refactor saving sdap entities

There was too much code duplication between
sdap_save_{user,group,netgroup}. This patch removes the most egregious ones.

5 files changed, 146 insertions, 179 deletions

diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index b1177e274..98291e6e2 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -1983,3 +1983,83 @@ done:
     return ret;
 }
 
+errno_t
+sdap_attrs_add_ldap_attr(struct sysdb_attrs *ldap_attrs,
+                         const char *attr_name,
+                         const char *attr_desc,
+                         bool multivalued,
+                         const char *name,
+                         struct sysdb_attrs *attrs)
+{
+    errno_t ret;
+    struct ldb_message_element *el;
+    const char *objname = name ?: "object";
+    const char *desc = attr_desc ?: attr_name;
+    unsigned int num_values, i;
+
+    ret = sysdb_attrs_get_el(ldap_attrs, attr_name, &el);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not get %s from the "
+              "list of the LDAP attributes [%d]: %s\n", ret, strerror(ret)));
+        return ret;
+    }
+
+    if (el->num_values == 0) {
+        DEBUG(SSSDBG_TRACE_INTERNAL, ("%s is not available "
+              "for [%s].\n", desc, objname));
+    } else {
+        num_values = multivalued ? el->num_values : 1;
+        for (i = 0; i < num_values; i++) {
+            DEBUG(SSSDBG_TRACE_INTERNAL, ("Adding %s [%s] to attributes "
+                  "of [%s].\n", desc, el->values[i].data, objname));
+
+            ret = sysdb_attrs_add_string(attrs, attr_name,
+                                         (const char *) el->values[i].data);
+            if (ret) {
+                return ret;
+            }
+        }
+    }
+
+    return EOK;
+}
+
+
+errno_t
+sdap_save_all_names(const char *name,
+                    struct sysdb_attrs *ldap_attrs,
+                    struct sysdb_attrs *attrs)
+{
+    const char **aliases = NULL;
+    errno_t ret;
+    TALLOC_CTX *tmp_ctx;
+    int i;
+
+    tmp_ctx = talloc_new(NULL);
+    if (!tmp_ctx) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    ret = sysdb_attrs_get_aliases(tmp_ctx, ldap_attrs, name, &aliases);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Failed to get the alias list"));
+        goto done;
+    }
+
+    for (i = 0; aliases[i]; i++) {
+        ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS,
+                                     aliases[i]);
+        if (ret) {
+            DEBUG(SSSDBG_OP_FAILURE, ("Failed to add alias [%s] into the "
+                                      "attribute list\n", aliases[i]));
+            goto done;
+        }
+    }
+
+    ret = EOK;
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
index 4ba2770c9..f53af1e01 100644
--- a/src/providers/ldap/sdap_async.h
+++ b/src/providers/ldap/sdap_async.h
@@ -189,4 +189,25 @@ errno_t sdap_check_aliases(struct sysdb_ctx *sysdb,
                            struct sss_domain_info *dom,
                            struct sdap_options *opts,
                            bool steal_memberships);
+
+errno_t
+sdap_attrs_add_ldap_attr(struct sysdb_attrs *ldap_attrs,
+                         const char *attr_name,
+                         const char *attr_desc,
+                         bool multivalued,
+                         const char *name,
+                         struct sysdb_attrs *attrs);
+
+#define sdap_attrs_add_string(ldap_attrs, attr_name, attr_desc, name, attrs) \
+        sdap_attrs_add_ldap_attr(ldap_attrs, attr_name, attr_desc, \
+                                 false, name, attrs)
+
+#define sdap_attrs_add_list(ldap_attrs, attr_name, attr_desc, name, attrs) \
+    sdap_attrs_add_ldap_attr(ldap_attrs, attr_name, attr_desc,   \
+                             true, name, attrs)
+
+errno_t sdap_save_all_names(const char *name,
+                            struct sysdb_attrs *ldap_attrs,
+                            struct sysdb_attrs *attrs);
+
 #endif /* _SDAP_ASYNC_H_ */
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index d27e03860..750ac998a 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -222,11 +222,9 @@ static int sdap_save_group(TALLOC_CTX *memctx,
     const char *name = NULL;
     gid_t gid;
     int ret;
-    int i;
     char *usn_value = NULL;
     TALLOC_CTX *tmpctx = NULL;
     bool posix_group;
-    const char **aliases = NULL;
 
     tmpctx = talloc_new(memctx);
     if (!tmpctx) {
@@ -282,38 +280,19 @@ static int sdap_save_group(TALLOC_CTX *memctx,
         /* Group ID OK */
     }
 
-    ret = sysdb_attrs_get_el(attrs, SYSDB_ORIG_DN, &el);
-    if (ret) {
+    ret = sdap_attrs_add_string(attrs, SYSDB_ORIG_DN, "original DN",
+                                name, group_attrs);
+    if (ret != EOK) {
         goto fail;
     }
-    if (el->num_values == 0) {
-        DEBUG(7, ("Original DN is not available for [%s].\n", name));
-    } else {
-        DEBUG(7, ("Adding original DN [%s] to attributes of [%s].\n",
-                  el->values[0].data, name));
-        ret = sysdb_attrs_add_string(group_attrs, SYSDB_ORIG_DN,
-                                     (const char *) el->values[0].data);
-        if (ret) {
-            goto fail;
-        }
-    }
 
-    ret = sysdb_attrs_get_el(attrs,
-                      opts->group_map[SDAP_AT_GROUP_MODSTAMP].sys_name, &el);
-    if (ret) {
+    ret = sdap_attrs_add_string(attrs,
+                            opts->group_map[SDAP_AT_GROUP_MODSTAMP].sys_name,
+                            "original mod-Timestamp",
+                            name, group_attrs);
+    if (ret != EOK) {
         goto fail;
     }
-    if (el->num_values == 0) {
-        DEBUG(7, ("Original mod-Timestamp is not available for [%s].\n",
-                  name));
-    } else {
-        ret = sysdb_attrs_add_string(group_attrs,
-                          opts->group_map[SDAP_AT_GROUP_MODSTAMP].sys_name,
-                          (const char*)el->values[0].data);
-        if (ret) {
-            goto fail;
-        }
-    }
 
     ret = sysdb_attrs_get_el(attrs,
                       opts->group_map[SDAP_AT_GROUP_USN].sys_name, &el);
@@ -369,20 +348,12 @@ static int sdap_save_group(TALLOC_CTX *memctx,
         }
     }
 
-    ret = sysdb_attrs_get_aliases(tmpctx, attrs, name, &aliases);
+    ret = sdap_save_all_names(name, attrs, group_attrs);
     if (ret != EOK) {
-        DEBUG(1, ("Failed to get the alias list\n"));
+        DEBUG(1, ("Failed to save user names\n"));
         goto fail;
     }
 
-    for (i = 0; aliases[i]; i++) {
-        ret = sysdb_attrs_add_string(group_attrs, SYSDB_NAME_ALIAS,
-                                        aliases[i]);
-        if (ret) {
-            goto fail;
-        }
-    }
-
     DEBUG(6, ("Storing info for group %s\n", name));
 
     ret = sdap_store_group_with_gid(ctx,
diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c
index 0f486dedd..88efc5e2a 100644
--- a/src/providers/ldap/sdap_async_netgroups.c
+++ b/src/providers/ldap/sdap_async_netgroups.c
@@ -48,7 +48,6 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx,
     const char *name = NULL;
     int ret;
     char *timestamp = NULL;
-    size_t c;
 
     ret = sysdb_attrs_get_el(attrs,
                              opts->netgroup_map[SDAP_AT_NETGROUP_NAME].sys_name,
@@ -66,21 +65,12 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx,
         goto fail;
     }
 
-    ret = sysdb_attrs_get_el(attrs, SYSDB_ORIG_DN, &el);
-    if (ret) {
+    ret = sdap_attrs_add_string(attrs, SYSDB_ORIG_DN,
+                                "original DN",
+                                name, netgroup_attrs);
+    if (ret != EOK) {
         goto fail;
     }
-    if (el->num_values == 0) {
-        DEBUG(7, ("Original DN is not available for [%s].\n", name));
-    } else {
-        DEBUG(7, ("Adding original DN [%s] to attributes of [%s].\n",
-                  el->values[0].data, name));
-        ret = sysdb_attrs_add_string(netgroup_attrs, SYSDB_ORIG_DN,
-                                     (const char *)el->values[0].data);
-        if (ret) {
-            goto fail;
-        }
-    }
 
     ret = sysdb_attrs_get_el(attrs,
                          opts->netgroup_map[SDAP_AT_NETGROUP_MODSTAMP].sys_name,
@@ -105,64 +95,27 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx,
         }
     }
 
-    ret = sysdb_attrs_get_el(attrs,
-                           opts->netgroup_map[SDAP_AT_NETGROUP_TRIPLE].sys_name,
-                           &el);
-    if (ret) {
+    ret = sdap_attrs_add_list(attrs,
+                        opts->netgroup_map[SDAP_AT_NETGROUP_TRIPLE].sys_name,
+                        "netgroup triple",
+                        name, netgroup_attrs);
+    if (ret != EOK) {
         goto fail;
     }
-    if (el->num_values == 0) {
-        DEBUG(7, ("No netgroup triples for netgroup [%s].\n", name));
-    } else {
-        for(c = 0; c < el->num_values; c++) {
-            ret = sysdb_attrs_add_string(netgroup_attrs,
-                           opts->netgroup_map[SDAP_AT_NETGROUP_TRIPLE].sys_name,
-                            (const char*)el->values[c].data);
-            if (ret) {
-                goto fail;
-            }
-        }
-    }
 
-    ret = sysdb_attrs_get_el(attrs,
-                       opts->netgroup_map[SDAP_AT_NETGROUP_MEMBER].sys_name,
-                       &el);
+    ret = sdap_attrs_add_list(attrs,
+                        opts->netgroup_map[SDAP_AT_NETGROUP_MEMBER].sys_name,
+                        "original members",
+                        name, netgroup_attrs);
     if (ret != EOK) {
         goto fail;
     }
-    if (el->num_values == 0) {
-        DEBUG(7, ("No original members for netgroup [%s]\n", name));
-
-    } else {
-        DEBUG(7, ("Adding original members to netgroup [%s]\n", name));
-        for(c = 0; c < el->num_values; c++) {
-            ret = sysdb_attrs_add_string(netgroup_attrs,
-                       opts->netgroup_map[SDAP_AT_NETGROUP_MEMBER].sys_name,
-                       (const char*)el->values[c].data);
-            if (ret) {
-                goto fail;
-            }
-        }
-    }
 
-
-    ret = sysdb_attrs_get_el(attrs, SYSDB_NETGROUP_MEMBER, &el);
+    ret = sdap_attrs_add_list(attrs, SYSDB_NETGROUP_MEMBER,
+                        "members", name, netgroup_attrs);
     if (ret != EOK) {
         goto fail;
     }
-    if (el->num_values == 0) {
-        DEBUG(7, ("No members for netgroup [%s]\n", name));
-
-    } else {
-        DEBUG(7, ("Adding members to netgroup [%s]\n", name));
-        for(c = 0; c < el->num_values; c++) {
-            ret = sysdb_attrs_add_string(netgroup_attrs, SYSDB_NETGROUP_MEMBER,
-                                         (const char*)el->values[c].data);
-            if (ret) {
-                goto fail;
-            }
-        }
-    }
 
     DEBUG(6, ("Storing info for netgroup %s\n", name));
 
diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c
index cf9a8d33c..c929e2048 100644
--- a/src/providers/ldap/sdap_async_users.c
+++ b/src/providers/ldap/sdap_async_users.c
@@ -29,7 +29,6 @@
 /* ==Save-User-Entry====================================================== */
 
 /* FIXME: support storing additional attributes */
-
 int sdap_save_user(TALLOC_CTX *memctx,
                    struct sysdb_ctx *ctx,
                    struct sdap_options *opts,
@@ -52,17 +51,14 @@ int sdap_save_user(TALLOC_CTX *memctx,
     struct sysdb_attrs *user_attrs;
     char *upn = NULL;
     size_t i;
-    char *val = NULL;
     int cache_timeout;
     char *usn_value = NULL;
-    size_t c;
     char **missing = NULL;
-    const char **aliases = NULL;
     TALLOC_CTX *tmpctx = NULL;
 
     DEBUG(9, ("Save user\n"));
 
-    tmpctx = talloc_new(memctx);
+    tmpctx = talloc_new(NULL);
     if (!tmpctx) {
         ret = ENOMEM;
         goto fail;
@@ -151,57 +147,27 @@ int sdap_save_user(TALLOC_CTX *memctx,
         goto fail;
     }
 
-    ret = sysdb_attrs_get_el(attrs, SYSDB_ORIG_DN, &el);
-    if (ret) {
+    ret = sdap_attrs_add_string(attrs, SYSDB_ORIG_DN,
+                                "original DN",
+                                name, user_attrs);
+    if (ret != EOK) {
         goto fail;
     }
-    if (el->num_values == 0) {
-        DEBUG(7, ("Original DN is not available for [%s].\n", name));
-    } else {
-        DEBUG(7, ("Adding original DN [%s] to attributes of [%s].\n",
-                  el->values[0].data, name));
-        ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_DN,
-                                     (const char *) el->values[0].data);
-        if (ret) {
-            goto fail;
-        }
-    }
 
-    ret = sysdb_attrs_get_el(attrs, SYSDB_MEMBEROF, &el);
-    if (ret) {
+    ret = sdap_attrs_add_list(attrs, SYSDB_MEMBEROF,
+                              "original memberOf",
+                              name, user_attrs);
+    if (ret != EOK) {
         goto fail;
     }
-    if (el->num_values == 0) {
-        DEBUG(7, ("Original memberOf is not available for [%s].\n",
-                  name));
-    } else {
-        DEBUG(7, ("Adding original memberOf attributes to [%s].\n",
-                  name));
-        for (i = 0; i < el->num_values; i++) {
-            ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF,
-                                         (const char *) el->values[i].data);
-            if (ret) {
-                goto fail;
-            }
-        }
-    }
 
-    ret = sysdb_attrs_get_el(attrs,
-                      opts->user_map[SDAP_AT_USER_MODSTAMP].sys_name, &el);
-    if (ret) {
+    ret = sdap_attrs_add_string(attrs,
+                            opts->user_map[SDAP_AT_USER_MODSTAMP].sys_name,
+                            "original mod-Timestamp",
+                            name, user_attrs);
+    if (ret != EOK) {
         goto fail;
     }
-    if (el->num_values == 0) {
-        DEBUG(7, ("Original mod-Timestamp is not available for [%s].\n",
-                  name));
-    } else {
-        ret = sysdb_attrs_add_string(user_attrs,
-                          opts->user_map[SDAP_AT_USER_MODSTAMP].sys_name,
-                          (const char*)el->values[0].data);
-        if (ret) {
-            goto fail;
-        }
-    }
 
     ret = sysdb_attrs_get_el(attrs,
                       opts->user_map[SDAP_AT_USER_USN].sys_name, &el);
@@ -218,7 +184,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
         if (ret) {
             goto fail;
         }
-        usn_value = talloc_strdup(memctx, (const char*)el->values[0].data);
+        usn_value = talloc_strdup(tmpctx, (const char*)el->values[0].data);
         if (!usn_value) {
             ret = ENOMEM;
             goto fail;
@@ -250,27 +216,11 @@ int sdap_save_user(TALLOC_CTX *memctx,
     }
 
     for (i = SDAP_FIRST_EXTRA_USER_AT; i < SDAP_OPTS_USER; i++) {
-        ret = sysdb_attrs_get_el(attrs, opts->user_map[i].sys_name, &el);
+        ret = sdap_attrs_add_list(attrs, opts->user_map[i].sys_name,
+                                  NULL, name, user_attrs);
         if (ret) {
             goto fail;
         }
-        if (el->num_values > 0) {
-            for (c = 0; c < el->num_values; c++) {
-                DEBUG(9, ("Adding [%s]=[%s] to user attributes.\n",
-                          opts->user_map[i].sys_name,
-                          (const char*) el->values[c].data));
-                val = talloc_strdup(user_attrs, (const char*) el->values[c].data);
-                if (val == NULL) {
-                    ret = ENOMEM;
-                    goto fail;
-                }
-                ret = sysdb_attrs_add_string(user_attrs,
-                                             opts->user_map[i].sys_name, val);
-                if (ret) {
-                    goto fail;
-                }
-            }
-        }
     }
 
     cache_timeout = dp_opt_get_int(opts->basic, SDAP_ENTRY_CACHE_TIMEOUT);
@@ -284,20 +234,12 @@ int sdap_save_user(TALLOC_CTX *memctx,
         }
     }
 
-    ret = sysdb_attrs_get_aliases(tmpctx, attrs, name, &aliases);
+    ret = sdap_save_all_names(name, attrs, user_attrs);
     if (ret != EOK) {
-        DEBUG(1, ("Failed to get the alias list"));
+        DEBUG(1, ("Failed to save user names\n"));
         goto fail;
     }
 
-    for (i = 0; aliases[i]; i++) {
-        ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS,
-                                     aliases[i]);
-        if (ret) {
-            goto fail;
-        }
-    }
-
     /* Make sure that any attributes we requested from LDAP that we
      * did not receive are also removed from the sysdb
      */
@@ -320,7 +262,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
     if (ret) goto fail;
 
     if (_usn_value) {
-        *_usn_value = usn_value;
+        *_usn_value = talloc_steal(memctx, usn_value);
     }
 
     talloc_steal(memctx, user_attrs);