>From 3d57af01ed379ff68549ce0e0f7f3162ae8146a8 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzidek@redhat.com>
Date: Fri, 19 Apr 2013 18:02:32 +0200
Subject: [PATCH 1/2] libsss_idmap: function to calculate range

Calculation of range for domains is moved from
sdap_idmap code to sss_idmap code. Some refactoring
have been done to allow this move.

https://fedorahosted.org/sssd/ticket/1844
---
 Makefile.am                       |   3 +-
 src/lib/idmap/sss_idmap.c         | 104 ++++++++++++++++++++++++++
 src/lib/idmap/sss_idmap.h         |  36 ++++++++-
 src/lib/idmap/sss_idmap_private.h |   3 +
 src/providers/ldap/sdap_idmap.c   | 152 +++++++++++---------------------------
 src/providers/ldap/sdap_idmap.h   |   8 --
 6 files changed, 188 insertions(+), 118 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index ef18022..17cd9d2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -581,7 +581,8 @@ libipa_hbac_la_LDFLAGS = \
 dist_pkgconfig_DATA += src/lib/idmap/sss_idmap.pc
 libsss_idmap_la_SOURCES = \
     src/lib/idmap/sss_idmap.c \
-    src/lib/idmap/sss_idmap_conv.c
+    src/lib/idmap/sss_idmap_conv.c \
+    src/util/murmurhash3.c
 libsss_idmap_la_LDFLAGS = \
     -version-info 0:1:0
 
diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index 1764b6f..3c57f3e 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -28,6 +28,7 @@
 
 #include "lib/idmap/sss_idmap.h"
 #include "lib/idmap/sss_idmap_private.h"
+#include "util/murmurhash3.h"
 
 #define SID_FMT "%s-%d"
 #define SID_STR_MAX_LEN 1024
@@ -180,6 +181,7 @@ bool is_domain_sid(const char *sid)
 enum idmap_error_code sss_idmap_init(idmap_alloc_func *alloc_func,
                                      void *alloc_pvt,
                                      idmap_free_func *free_func,
+                                     struct sss_idmap_opts *idmap_opts,
                                      struct sss_idmap_ctx **_ctx)
 {
     struct sss_idmap_ctx *ctx;
@@ -198,6 +200,10 @@ enum idmap_error_code sss_idmap_init(idmap_alloc_func *alloc_func,
     ctx->alloc_pvt = alloc_pvt;
     ctx->free_func = (free_func == NULL) ? default_free : free_func;
 
+    /* There are no pointers in the sss_idmap_opts structure, so shallow copy
+     * is fine. */
+    ctx->idmap_opts = *idmap_opts;
+
     *_ctx = ctx;
 
     return IDMAP_SUCCESS;
@@ -225,6 +231,84 @@ enum idmap_error_code sss_idmap_free(struct sss_idmap_ctx *ctx)
     return IDMAP_SUCCESS;
 }
 
+enum idmap_error_code sss_idmap_calculate_range(struct sss_idmap_ctx *ctx,
+                                                const char *dom_sid,
+                                                id_t slice_num,
+                                                struct sss_idmap_range *_range)
+{
+    id_t max_slices;
+    id_t orig_slice;
+    id_t new_slice;
+    uint32_t hash_val;
+    struct idmap_domain_info *dom;
+    id_t idmap_lower = ctx->idmap_opts.idmap_lower;
+    id_t idmap_upper = ctx->idmap_opts.idmap_upper;
+    id_t rangesize = ctx->idmap_opts.rangesize;
+    bool autorid_mode = ctx->idmap_opts.autorid_mode;
+
+    max_slices = (idmap_upper - idmap_lower) / rangesize;
+
+    if (slice_num != -1) {
+        /* The slice is being set explicitly.
+         * This may happen at system startup when we're loading
+         * previously-determined slices. In the future, we may also
+         * permit configuration to select the slice for a domain
+         * explicitly.
+         */
+        new_slice = slice_num;
+    } else {
+        /* If slice is -1, we're being asked to pick a new slice */
+
+        if (autorid_mode) {
+            /* In autorid compatibility mode, always start at 0 and find the
+             * first free value.
+             */
+            orig_slice = 0;
+        } else {
+            /* Hash the domain sid string */
+            hash_val = murmurhash3(dom_sid, strlen(dom_sid), 0xdeadbeef);
+
+            /* Now get take the modulus of the hash val and the max_slices
+             * to determine its optimal position in the range.
+             */
+            new_slice = hash_val % max_slices;
+            orig_slice = new_slice;
+        }
+        /* Verify that this slice is not already in use */
+        do {
+            for (dom = ctx->idmap_domain_info; dom != NULL; dom = dom->next) {
+                if (dom->range->slice_num == new_slice) {
+                    /* This slice number matches one already registered
+                     * We'll try the next available slot
+                     */
+                    new_slice++;
+                    if (new_slice > max_slices) {
+                        /* loop around to the beginning if necessary */
+                        new_slice = 0;
+                    }
+                    break;
+                }
+            }
+
+            /* Keep trying until dom is NULL (meaning we got to the end
+             * without matching) or we have run out of slices and gotten
+             * back to the first one we tried.
+             */
+        } while (dom && new_slice != orig_slice);
+
+        if (dom) {
+            /* We looped all the way through and found no empty slots */
+            return IDMAP_OUT_OF_SLICES;
+        }
+    }
+
+    _range->min = (rangesize * new_slice) + idmap_lower;
+    _range->max = _range->min + rangesize;
+    _range->slice_num = new_slice;
+
+    return IDMAP_SUCCESS;
+}
+
 enum idmap_error_code sss_idmap_add_domain(struct sss_idmap_ctx *ctx,
                                            const char *domain_name,
                                            const char *domain_sid,
@@ -511,3 +595,23 @@ done:
     return err;
 
 }
+
+bool sss_idmap_ctx_get_autorid(struct sss_idmap_ctx *ctx)
+{
+    return ctx->idmap_opts.autorid_mode;
+}
+
+id_t sss_idmap_ctx_get_lower(struct sss_idmap_ctx *ctx)
+{
+    return ctx->idmap_opts.idmap_lower;
+}
+
+id_t sss_idmap_ctx_get_upper(struct sss_idmap_ctx *ctx)
+{
+    return ctx->idmap_opts.idmap_upper;
+}
+
+id_t sss_idmap_ctx_get_rangesize(struct sss_idmap_ctx *ctx)
+{
+    return ctx->idmap_opts.rangesize;
+}
diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h
index ced7074..1ed4251 100644
--- a/src/lib/idmap/sss_idmap.h
+++ b/src/lib/idmap/sss_idmap.h
@@ -71,7 +71,10 @@ enum idmap_error_code {
     IDMAP_NO_RANGE,
 
     /** The provided SID is a built-in one */
-    IDMAP_BUILTIN_SID
+    IDMAP_BUILTIN_SID,
+
+    /* No more free slices */
+    IDMAP_OUT_OF_SLICES
 };
 
 /**
@@ -88,6 +91,7 @@ typedef void (idmap_free_func)(void *ptr, void *pvt);
 struct sss_idmap_range {
     uint32_t min;
     uint32_t max;
+    id_t slice_num;
 };
 
 /**
@@ -101,6 +105,30 @@ struct sss_dom_sid;
 struct sss_idmap_ctx;
 
 /**
+ * Structure for configurable options.
+ * This structure is passed as parameter to sss_idmap_init, to reduce the
+ * number of parameters. It's values are later accessible via getters.
+ */
+struct sss_idmap_opts {
+    /* true if autorid compatibility mode is used */
+    bool autorid_mode;
+
+    /* smallest available id (for all domains) */
+    id_t idmap_lower;
+
+    /* highest available id (for all domains) */
+    id_t idmap_upper;
+
+    /* number of available UIDs (for single domain) */
+    id_t rangesize;
+};
+
+bool sss_idmap_ctx_get_autorid(struct sss_idmap_ctx *ctx);
+id_t sss_idmap_ctx_get_lower(struct sss_idmap_ctx *ctx);
+id_t sss_idmap_ctx_get_upper(struct sss_idmap_ctx *ctx);
+id_t sss_idmap_ctx_get_rangesize(struct sss_idmap_ctx *ctx);
+
+/**
  * Placeholder for Samba's struct dom_sid. Consumers of libsss_idmap should
  * include an appropriate Samba header file to define struct dom_sid. We use
  * it here to avoid a hard dependency on Samba devel packages.
@@ -123,8 +151,14 @@ struct dom_sid;
 enum idmap_error_code sss_idmap_init(idmap_alloc_func *alloc_func,
                                      void *alloc_pvt,
                                      idmap_free_func *free_func,
+                                     struct sss_idmap_opts *opts,
                                      struct sss_idmap_ctx **ctx);
 
+enum idmap_error_code sss_idmap_calculate_range(struct sss_idmap_ctx *ctx,
+                                                const char *dom_sid,
+                                                id_t slice_num,
+                                                struct sss_idmap_range *range);
+
 /**
  * @brief Add a domain to the idmap context
  *
diff --git a/src/lib/idmap/sss_idmap_private.h b/src/lib/idmap/sss_idmap_private.h
index bdb5289..3defe41 100644
--- a/src/lib/idmap/sss_idmap_private.h
+++ b/src/lib/idmap/sss_idmap_private.h
@@ -25,6 +25,8 @@
 #ifndef SSS_IDMAP_PRIVATE_H_
 #define SSS_IDMAP_PRIVATE_H_
 
+#include "providers/ldap/sdap_idmap.h"
+
 #define CHECK_IDMAP_CTX(ctx, ret) do { \
     if (ctx == NULL || ctx->alloc_func == NULL || ctx->free_func == NULL) { \
         return ret; \
@@ -35,6 +37,7 @@ struct sss_idmap_ctx {
     idmap_alloc_func *alloc_func;
     void *alloc_pvt;
     idmap_free_func *free_func;
+    struct sss_idmap_opts idmap_opts;
     struct idmap_domain_info *idmap_domain_info;
 };
 
diff --git a/src/providers/ldap/sdap_idmap.c b/src/providers/ldap/sdap_idmap.c
index 0db3265..0537142 100644
--- a/src/providers/ldap/sdap_idmap.c
+++ b/src/providers/ldap/sdap_idmap.c
@@ -50,8 +50,14 @@ sdap_idmap_init(TALLOC_CTX *mem_ctx,
     const char *dom_name;
     const char *sid_str;
     id_t slice_num;
+    id_t idmap_lower;
+    id_t idmap_upper;
+    id_t rangesize;
+    bool autorid_mode;
     struct sdap_idmap_ctx *idmap_ctx = NULL;
     struct sysdb_ctx *sysdb = id_ctx->be->domain->sysdb;
+    struct sss_idmap_opts idmap_opts;
+
 
     tmp_ctx = talloc_new(NULL);
     if (!tmp_ctx) return ENOMEM;
@@ -63,9 +69,40 @@ sdap_idmap_init(TALLOC_CTX *mem_ctx,
     }
     idmap_ctx->id_ctx = id_ctx;
 
+    idmap_lower = dp_opt_get_int(idmap_ctx->id_ctx->opts->basic,
+                                 SDAP_IDMAP_LOWER);
+    idmap_upper = dp_opt_get_int(idmap_ctx->id_ctx->opts->basic,
+                                 SDAP_IDMAP_UPPER);
+    rangesize = dp_opt_get_int(idmap_ctx->id_ctx->opts->basic,
+                               SDAP_IDMAP_RANGESIZE);
+    autorid_mode = dp_opt_get_bool(idmap_ctx->id_ctx->opts->basic,
+                                   SDAP_IDMAP_AUTORID_COMPAT);
+
+    /* Validate that the values make sense */
+    if (rangesize <= 0
+            || idmap_upper <= idmap_lower
+            || (idmap_upper-idmap_lower) < rangesize)
+    {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Invalid settings for range selection: [%d][%d][%d]\n",
+               idmap_lower, idmap_upper, rangesize));
+        ret = EINVAL;
+    }
+
+    if (((idmap_upper - idmap_lower) % rangesize) != 0) {
+        DEBUG(SSSDBG_CONF_SETTINGS,
+              ("Range size does not divide evenly. Uppermost range will "
+               "not be used\n"));
+    }
+
     /* Initialize the map */
+    idmap_opts.autorid_mode = autorid_mode;
+    idmap_opts.idmap_lower = idmap_lower;
+    idmap_opts.idmap_upper = idmap_upper;
+    idmap_opts.rangesize = rangesize;
     err = sss_idmap_init(sdap_idmap_talloc, idmap_ctx,
                          sdap_idmap_talloc_free,
+                         &idmap_opts,
                          &idmap_ctx->map);
     if (err != IDMAP_SUCCESS) {
         DEBUG(SSSDBG_CRIT_FAILURE,
@@ -182,117 +219,19 @@ sdap_idmap_add_domain(struct sdap_idmap_ctx *idmap_ctx,
                       id_t slice)
 {
     errno_t ret;
-    struct sdap_idmap_slice *new_slice;
-    id_t idmap_lower;
-    id_t idmap_upper;
-    id_t rangesize;
-    id_t max_slices;
-    id_t orig_slice;
-    uint32_t hash_val;
-    struct sdap_idmap_slice *s;
     struct sss_idmap_range range;
     enum idmap_error_code err;
+    id_t idmap_upper = sss_idmap_ctx_get_upper(idmap_ctx->map);
 
-    idmap_lower = dp_opt_get_int(idmap_ctx->id_ctx->opts->basic,
-                                 SDAP_IDMAP_LOWER);
-    idmap_upper = dp_opt_get_int(idmap_ctx->id_ctx->opts->basic,
-                                 SDAP_IDMAP_UPPER);
-    rangesize = dp_opt_get_int(idmap_ctx->id_ctx->opts->basic,
-                               SDAP_IDMAP_RANGESIZE);
 
-    /* Validate that the values make sense */
-    if (rangesize <= 0
-            || idmap_upper <= idmap_lower
-            || (idmap_upper-idmap_lower) < rangesize)
-    {
+    ret = sss_idmap_calculate_range(idmap_ctx->map, dom_sid, slice, &range);
+    if (ret != IDMAP_SUCCESS) {
         DEBUG(SSSDBG_CRIT_FAILURE,
-              ("Invalid settings for range selection: [%d][%d][%d]\n",
-               idmap_lower, idmap_upper, rangesize));
-        return EINVAL;
-    }
-
-    max_slices = (idmap_upper - idmap_lower) / rangesize;
-    if (((idmap_upper - idmap_lower) % rangesize) != 0) {
-        DEBUG(SSSDBG_CONF_SETTINGS,
-              ("Range size does not divide evenly. Uppermost range will "
-               "not be used\n"));
-    }
-
-    new_slice = talloc_zero(idmap_ctx, struct sdap_idmap_slice);
-    if (!new_slice) return ENOMEM;
-
-    if (slice != -1) {
-        /* The slice is being set explicitly.
-         * This may happen at system startup when we're loading
-         * previously-determined slices. In the future, we may also
-         * permit configuration to select the slice for a domain
-         * explicitly.
-         */
-        new_slice->slice_num = slice;
-    } else {
-        /* If slice is -1, we're being asked to pick a new slice */
-
-        if (dp_opt_get_bool(idmap_ctx->id_ctx->opts->basic, SDAP_IDMAP_AUTORID_COMPAT)) {
-            /* In autorid compatibility mode, always start at 0 and find the first
-             * free value.
-             */
-            orig_slice = 0;
-        } else {
-            /* Hash the domain sid string */
-            hash_val = murmurhash3(dom_sid, strlen(dom_sid), 0xdeadbeef);
-
-            /* Now get take the modulus of the hash val and the max_slices
-             * to determine its optimal position in the range.
-             */
-            new_slice->slice_num = hash_val % max_slices;
-            orig_slice = new_slice->slice_num;
-        }
-        /* Verify that this slice is not already in use */
-        do {
-            DLIST_FOR_EACH(s, idmap_ctx->slices) {
-                if (s->slice_num == new_slice->slice_num) {
-                    /* This slice number matches one already registered
-                     * We'll try the next available slot
-                     */
-                    new_slice->slice_num++;
-                    if (new_slice->slice_num > max_slices) {
-                        /* loop around to the beginning if necessary */
-                        new_slice->slice_num = 0;
-                    }
-                    break;
-                }
-            }
-
-            /* Keep trying until s is NULL (meaning we got to the end
-             * without matching) or we have run out of slices and gotten
-             * back to the first one we tried.
-             */
-        } while (s && new_slice->slice_num != orig_slice);
-
-        if (s) {
-            /* We looped all the way through and found no empty slots */
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  ("Could not add domain [%s]: no free slices\n",
-                   dom_name));
-            ret = ENOSPC;
-            goto done;
-        }
+              ("Failed to calculate range for domain [%s]: [%d]\n", dom_name,
+               ret));
+        ret = EIO;
     }
 
-    DEBUG(SSSDBG_CONF_SETTINGS,
-          ("Adding domain [%s] as slice [%d]\n",
-           dom_name, new_slice->slice_num));
-
-    DLIST_ADD_END(idmap_ctx->slices, new_slice, struct sdap_idmap_slice *);
-    /* Not adding a destructor to remove from this list, because it
-     * should never be possible. Removal from this list can only
-     * destabilize the system.
-     */
-
-    /* Create a range object to add to the mapping */
-    range.min = (rangesize * new_slice->slice_num) + idmap_lower;
-    range.max = range.min + rangesize;
-
     if (range.max > idmap_upper) {
         /* This should never happen */
         DEBUG(SSSDBG_CRIT_FAILURE,
@@ -316,11 +255,8 @@ sdap_idmap_add_domain(struct sdap_idmap_ctx *idmap_ctx,
     ret = sysdb_idmap_store_mapping(idmap_ctx->id_ctx->be->domain->sysdb,
                                     idmap_ctx->id_ctx->be->domain,
                                     dom_name, dom_sid,
-                                    new_slice->slice_num);
+                                    range.slice_num);
 done:
-    if (ret != EOK) {
-        talloc_free(new_slice);
-    }
     return ret;
 }
 
diff --git a/src/providers/ldap/sdap_idmap.h b/src/providers/ldap/sdap_idmap.h
index 99f2ad9..2e2123f 100644
--- a/src/providers/ldap/sdap_idmap.h
+++ b/src/providers/ldap/sdap_idmap.h
@@ -26,16 +26,8 @@
 #include "src/providers/ldap/sdap.h"
 #include "src/providers/ldap/ldap_common.h"
 
-struct sdap_idmap_slice {
-    struct sdap_idmap_slice *prev;
-    struct sdap_idmap_slice *next;
-
-    id_t slice_num;
-};
-
 struct sdap_idmap_ctx {
     struct sss_idmap_ctx *map;
-    struct sdap_idmap_slice *slices;
 
     struct sdap_id_ctx *id_ctx;
 };
-- 
1.7.11.2

