ldb: remove unpack only attr list functionality

Unpack functions currently take an attribute list to restrict the set of
attributes to be returned in the constructed message. This
functionality is never used and complicates implementation of
forthcoming new pack format. This patch removes that functionality.
Using the unpack function then filtering the result turns
out not to be any slower.

NOTE: Configure with --abi-check-disable to build this commit. This
patch is part of a set of LDB ABI changes, and the version update is
done on the last commit.

Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>

diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
index 24c506554b26aecc9235cbd09738bc8c22df6ca0..5360a36ccccfa5c50b29a9016f75d87e4848406a 100644 (file)
--- a/lib/ldb/common/ldb_pack.c
+++ b/lib/ldb/common/ldb_pack.c
@@ -180,50 +180,16 @@ int ldb_pack_data(struct ldb_context *ldb,
        return 0;
 }
 
-static bool ldb_consume_element_data(uint8_t **pp, size_t *premaining)
-{
-       unsigned int remaining = *premaining;
-       uint8_t *p = *pp;
-       uint32_t num_values = pull_uint32(p, 0);
-       uint32_t j, len;
-
-       p += 4;
-       if (remaining < 4) {
-               return false;
-       }
-       remaining -= 4;
-       for (j = 0; j < num_values; j++) {
-               len = pull_uint32(p, 0);
-               if (remaining < 5) {
-                       return false;
-               }
-               remaining -= 5;
-               if (len > remaining) {
-                       return false;
-               }
-               remaining -= len;
-               p += len + 4 + 1;
-       }
-
-       *premaining = remaining;
-       *pp = p;
-       return true;
-}
-
-
 /*
  * Unpack a ldb message from a linear buffer in ldb_val
  *
  * Providing a list of attributes to this function allows selective unpacking.
  * Giving a NULL list (or a list_size of 0) unpacks all the attributes.
  */
-int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
-                                        const struct ldb_val *data,
-                                        struct ldb_message *message,
-                                        const char * const *list,
-                                        unsigned int list_size,
-                                        unsigned int flags,
-                                        unsigned int *nb_elements_in_db)
+int ldb_unpack_data_flags(struct ldb_context *ldb,
+                         const struct ldb_val *data,
+                         struct ldb_message *message,
+                         unsigned int flags)
 {
        uint8_t *p;
        size_t remaining;
@@ -232,13 +198,8 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
        unsigned format;
        unsigned int nelem = 0;
        size_t len;
-       unsigned int found = 0;
        struct ldb_val *ldb_val_single_array = NULL;
 
-       if (list == NULL) {
-               list_size = 0;
-       }
-
        message->elements = NULL;
 
        p = data->data;
@@ -250,9 +211,6 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
        format = pull_uint32(p, 0);
        message->num_elements = pull_uint32(p, 4);
        p += 8;
-       if (nb_elements_in_db) {
-               *nb_elements_in_db = message->num_elements;
-       }
 
        remaining = data->length - 8;
 
@@ -364,44 +322,6 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
                }
                attr = (char *)p;
 
-               /*
-                * The general idea is to reduce allocations by skipping over
-                * attributes that we do not actually care about.
-                *
-                * This is a bit expensive but normally the list is pretty small
-                * also the cost of freeing unused attributes is quite important
-                * and can dwarf the cost of looping.
-                */
-               if (list_size != 0) {
-                       bool keep = false;
-                       unsigned int h;
-
-                       /*
-                        * We know that p has a \0 terminator before the
-                        * end of the buffer due to the check above.
-                        */
-                       for (h = 0; h < list_size && found < list_size; h++) {
-                               if (ldb_attr_cmp(attr, list[h]) == 0) {
-                                       keep = true;
-                                       found++;
-                                       break;
-                               }
-                       }
-
-                       if (!keep) {
-                               if (remaining < (attr_len + 1)) {
-                                       errno = EIO;
-                                       goto failed;
-                               }
-                               remaining -= attr_len + 1;
-                               p += attr_len + 1;
-                               if (!ldb_consume_element_data(&p, &remaining)) {
-                                       errno = EIO;
-                                       goto failed;
-                               }
-                               continue;
-                       }
-               }
                element = &message->elements[nelem];
                element->name = attr;
                element->flags = 0;
@@ -471,7 +391,7 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
 
        if (remaining != 0) {
                ldb_debug(ldb, LDB_DEBUG_ERROR,
-                         "Error: %zu bytes unread in ldb_unpack_data_only_attr_list",
+                         "Error: %zu bytes unread in ldb_unpack_data_flags",
                          remaining);
        }
 
@@ -485,32 +405,13 @@ failed:
 /*
  * Unpack a ldb message from a linear buffer in ldb_val
  *
- * Providing a list of attributes to this function allows selective unpacking.
- * Giving a NULL list (or a list_size of 0) unpacks all the attributes.
- *
  * Free with ldb_unpack_data_free()
  */
-int ldb_unpack_data_only_attr_list(struct ldb_context *ldb,
-                                  const struct ldb_val *data,
-                                  struct ldb_message *message,
-                                  const char * const *list,
-                                  unsigned int list_size,
-                                  unsigned int *nb_elements_in_db)
-{
-       return ldb_unpack_data_only_attr_list_flags(ldb,
-                                                   data,
-                                                   message,
-                                                   list,
-                                                   list_size,
-                                                   0,
-                                                   nb_elements_in_db);
-}
-
 int ldb_unpack_data(struct ldb_context *ldb,
                    const struct ldb_val *data,
                    struct ldb_message *message)
 {
-       return ldb_unpack_data_only_attr_list(ldb, data, message, NULL, 0, NULL);
+       return ldb_unpack_data_flags(ldb, data, message, 0);
 }
 
 /*

diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 8fa506032d7e0dd42b6379461c877c02fb3b856d..759a54a3169f91de091df2cf89b6da5a3a501901 100644 (file)
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -512,16 +512,7 @@ int ldb_pack_data(struct ldb_context *ldb,
                  struct ldb_val *data);
 /*
  * Unpack a ldb message from a linear buffer in ldb_val
- *
- * Providing a list of attributes to this function allows selective unpacking.
- * Giving a NULL list (or a list_size of 0) unpacks all the attributes.
  */
-int ldb_unpack_data_only_attr_list(struct ldb_context *ldb,
-                                  const struct ldb_val *data,
-                                  struct ldb_message *message,
-                                  const char* const * list,
-                                  unsigned int list_size,
-                                  unsigned int *nb_attributes_indb);
 int ldb_unpack_data(struct ldb_context *ldb,
                    const struct ldb_val *data,
                    struct ldb_message *message);
@@ -548,13 +539,10 @@ int ldb_filter_attrs(struct ldb_context *ldb,
  * are unpacked or returned.
  *
  */
-int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
-                                        const struct ldb_val *data,
-                                        struct ldb_message *message,
-                                        const char * const *list,
-                                        unsigned int list_size,
-                                        unsigned int flags,
-                                        unsigned int *nb_elements_in_db);
+int ldb_unpack_data_flags(struct ldb_context *ldb,
+                         const struct ldb_val *data,
+                         struct ldb_message *message,
+                         unsigned int flags);
 
 /* currently unused (was NO_DATA_ALLOC)      0x0001 */
 #define LDB_UNPACK_DATA_FLAG_NO_DN           0x0002

diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
index cc8e4d73e7af0fa555a6ab6519582b9c96ac4e14..e1ca1cd243674c209beaa382ce4e16f55bf28774 100644 (file)
--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
@@ -1669,11 +1669,8 @@ static int traverse_range_index(struct ldb_kv_private *ldb_kv,
 
        msg = ldb_msg_new(module);
 
-       ctx->error = ldb_unpack_data_only_attr_list_flags(ldb, &data,
-                                                         msg,
-                                                         NULL, 0,
-                                                         LDB_UNPACK_DATA_FLAG_NO_DN,
-                                                         NULL);
+       ctx->error = ldb_unpack_data_flags(ldb, &data, msg,
+                                          LDB_UNPACK_DATA_FLAG_NO_DN);
 
        if (ctx->error != LDB_SUCCESS) {
                talloc_free(msg);

diff --git a/lib/ldb/ldb_key_value/ldb_kv_search.c b/lib/ldb/ldb_key_value/ldb_kv_search.c
index 7a1567b34444339aaa0fd4d99a9aa69bb352cf45..ae576724426d036631ef7be58b2ff2f846dff6f6 100644 (file)
--- a/lib/ldb/ldb_key_value/ldb_kv_search.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_search.c
@@ -150,11 +150,8 @@ static int ldb_kv_parse_data_unpack(struct ldb_val key,
                }
        }
 
-       ret = ldb_unpack_data_only_attr_list_flags(ldb, &data_parse,
-                                                  ctx->msg,
-                                                  NULL, 0,
-                                                  ctx->unpack_flags,
-                                                  NULL);
+       ret = ldb_unpack_data_flags(ldb, &data_parse,
+                                   ctx->msg, ctx->unpack_flags);
        if (ret == -1) {
                if (data_parse.data != data.data) {
                        talloc_free(data_parse.data);
@@ -318,7 +315,6 @@ static int search_func(struct ldb_kv_private *ldb_kv,
        struct ldb_message *msg, *filtered_msg;
        int ret;
        bool matched;
-       unsigned int nb_elements_in_db;
 
        ac = talloc_get_type(state, struct ldb_kv_context);
        ldb = ldb_module_get_ctx(ac->module);
@@ -351,11 +347,8 @@ static int search_func(struct ldb_kv_private *ldb_kv,
        }
 
        /* unpack the record */
-       ret = ldb_unpack_data_only_attr_list_flags(ldb, &val,
-                                                  msg,
-                                                  NULL, 0,
-                                                  LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC,
-                                                  &nb_elements_in_db);
+       ret = ldb_unpack_data_flags(ldb, &val, msg,
+                                   LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC);
        if (ret == -1) {
                talloc_free(msg);
                ac->error = LDB_ERR_OPERATIONS_ERROR;

diff --git a/source4/torture/ldb/ldb.c b/source4/torture/ldb/ldb.c
index dfdce73d64f7954b369d4f3afc12e3fad1ddd973..bc3d0ac4b1a5c5346e6e10ca019fba2cdae6bc26 100644 (file)
--- a/source4/torture/ldb/ldb.c
+++ b/source4/torture/ldb/ldb.c
@@ -1111,7 +1111,6 @@ static bool torture_ldb_unpack_flags(struct torture_context *torture)
        struct ldb_message *msg = ldb_msg_new(mem_ctx);
        const char *ldif_text = dda1d01d_ldif;
        struct ldb_ldif ldif;
-       unsigned int nb_elements_in_db;
 
        ldb = samba_ldb_init(mem_ctx, torture->ev, NULL, NULL, NULL);
        torture_assert(torture,
@@ -1119,11 +1118,8 @@ static bool torture_ldb_unpack_flags(struct torture_context *torture)
                       "Failed to init ldb");
 
        torture_assert_int_equal(torture,
-                                ldb_unpack_data_only_attr_list_flags(ldb, &data,
-                                                                     msg,
-                                                                     NULL, 0,
-                                                                     LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC,
-                                                                     &nb_elements_in_db),
+                                ldb_unpack_data_flags(ldb, &data, msg,
+                                       LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC),
                                 0,
                                 "ldb_unpack_data failed");
 
@@ -1136,11 +1132,8 @@ static bool torture_ldb_unpack_flags(struct torture_context *torture)
                                 "ldif form differs from binary form");
 
        torture_assert_int_equal(torture,
-                                ldb_unpack_data_only_attr_list_flags(ldb, &data,
-                                                                     msg,
-                                                                     NULL, 0,
-                                                                     LDB_UNPACK_DATA_FLAG_NO_DN,
-                                                                     &nb_elements_in_db),
+                                ldb_unpack_data_flags(ldb, &data, msg,
+                                               LDB_UNPACK_DATA_FLAG_NO_DN),
                                 0,
                                 "ldb_unpack_data failed");