ldb: move ldb_kv's filter into pack code

This patch moves ldb_kv's filter code into the pack code to replace
'only attr list' functionality which will be removed in forthcoming
commit. Unpacking data then filtering the result is not any slower
than the removed 'only attr list' approach.
'only attr list' test repurposed to test unpack -> filter flow.

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 60242e178f62b193eae053db88709533316ffd5b..24c506554b26aecc9235cbd09738bc8c22df6ca0 100644 (file)
--- a/lib/ldb/common/ldb_pack.c
+++ b/lib/ldb/common/ldb_pack.c
@@ -512,3 +512,157 @@ int ldb_unpack_data(struct ldb_context *ldb,
 {
        return ldb_unpack_data_only_attr_list(ldb, data, message, NULL, 0, NULL);
 }
+
+/*
+  add the special distinguishedName element
+*/
+static int msg_add_distinguished_name(struct ldb_message *msg)
+{
+       const char *dn_attr = "distinguishedName";
+       char *dn = NULL;
+
+       if (ldb_msg_find_element(msg, dn_attr)) {
+               /*
+                * This should not happen, but this is
+                * existing behaviour...
+                */
+               return LDB_SUCCESS;
+       }
+
+       dn = ldb_dn_alloc_linearized(msg, msg->dn);
+       if (dn == NULL) {
+               return LDB_ERR_OPERATIONS_ERROR;
+       }
+
+       return ldb_msg_add_steal_string(msg, dn_attr, dn);
+}
+
+/*
+ * filter the specified list of attributes from msg,
+ * adding requested attributes, and perhaps all for *,
+ * but not the DN to filtered_msg.
+ */
+int ldb_filter_attrs(struct ldb_context *ldb,
+                    const struct ldb_message *msg,
+                    const char *const *attrs,
+                    struct ldb_message *filtered_msg)
+{
+       unsigned int i;
+       bool keep_all = false;
+       bool add_dn = false;
+       uint32_t num_elements;
+       uint32_t elements_size;
+
+       if (attrs) {
+               /* check for special attrs */
+               for (i = 0; attrs[i]; i++) {
+                       int cmp = strcmp(attrs[i], "*");
+                       if (cmp == 0) {
+                               keep_all = true;
+                               break;
+                       }
+                       cmp = ldb_attr_cmp(attrs[i], "distinguishedName");
+                       if (cmp == 0) {
+                               add_dn = true;
+                       }
+               }
+       } else {
+               keep_all = true;
+       }
+
+       if (keep_all) {
+               add_dn = true;
+               elements_size = msg->num_elements + 1;
+
+       /* Shortcuts for the simple cases */
+       } else if (add_dn && i == 1) {
+               if (msg_add_distinguished_name(filtered_msg) != 0) {
+                       goto failed;
+               }
+               return 0;
+       } else if (i == 0) {
+               return 0;
+
+       /* Otherwise we are copying at most as many element as we have attributes */
+       } else {
+               elements_size = i;
+       }
+
+       filtered_msg->elements = talloc_array(filtered_msg,
+                                             struct ldb_message_element,
+                                             elements_size);
+       if (filtered_msg->elements == NULL) goto failed;
+
+       num_elements = 0;
+
+       for (i = 0; i < msg->num_elements; i++) {
+               struct ldb_message_element *el = &msg->elements[i];
+               struct ldb_message_element *el2 = &filtered_msg->elements[num_elements];
+               unsigned int j;
+
+               if (keep_all == false) {
+                       bool found = false;
+                       for (j = 0; attrs[j]; j++) {
+                               int cmp = ldb_attr_cmp(el->name, attrs[j]);
+                               if (cmp == 0) {
+                                       found = true;
+                                       break;
+                               }
+                       }
+                       if (found == false) {
+                               continue;
+                       }
+               }
+               *el2 = *el;
+               el2->name = talloc_strdup(filtered_msg->elements,
+                                         el->name);
+               if (el2->name == NULL) {
+                       goto failed;
+               }
+               el2->values = talloc_array(filtered_msg->elements,
+                                          struct ldb_val, el->num_values);
+               if (el2->values == NULL) {
+                       goto failed;
+               }
+               for (j=0;j<el->num_values;j++) {
+                       el2->values[j] = ldb_val_dup(el2->values, &el->values[j]);
+                       if (el2->values[j].data == NULL && el->values[j].length != 0) {
+                               goto failed;
+                       }
+               }
+               num_elements++;
+
+               /* Pidginhole principle: we can't have more elements
+                * than the number of attributes if they are unique in
+                * the DB */
+               if (num_elements > elements_size) {
+                       goto failed;
+               }
+       }
+
+       filtered_msg->num_elements = num_elements;
+
+       if (add_dn) {
+               if (msg_add_distinguished_name(filtered_msg) != 0) {
+                       goto failed;
+               }
+       }
+
+       if (filtered_msg->num_elements > 0) {
+               filtered_msg->elements
+                       = talloc_realloc(filtered_msg,
+                                        filtered_msg->elements,
+                                        struct ldb_message_element,
+                                        filtered_msg->num_elements);
+               if (filtered_msg->elements == NULL) {
+                       goto failed;
+               }
+       } else {
+               talloc_free(filtered_msg->elements);
+               filtered_msg->elements = NULL;
+       }
+
+       return 0;
+failed:
+       return -1;
+}

diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index bf4879d1ca5dbaa91ca019ca85b892e5670fb28b..8fa506032d7e0dd42b6379461c877c02fb3b856d 100644 (file)
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -525,6 +525,16 @@ int ldb_unpack_data_only_attr_list(struct ldb_context *ldb,
 int ldb_unpack_data(struct ldb_context *ldb,
                    const struct ldb_val *data,
                    struct ldb_message *message);
+
+/*
+ * filter the specified list of attributes from msg,
+ * adding requested attributes, and perhaps all for *,
+ * but not the DN to filtered_msg.
+ */
+int ldb_filter_attrs(struct ldb_context *ldb,
+                    const struct ldb_message *msg,
+                    const char *const *attrs,
+                    struct ldb_message *filtered_msg);
 /*
  * Unpack a ldb message from a linear buffer in ldb_val
  *

diff --git a/lib/ldb/ldb_key_value/ldb_kv_search.c b/lib/ldb/ldb_key_value/ldb_kv_search.c
index 834036845c3e9c1faf5169188a9b2855995746f4..7a1567b34444339aaa0fd4d99a9aa69bb352cf45 100644 (file)
--- a/lib/ldb/ldb_key_value/ldb_kv_search.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_search.c
@@ -33,31 +33,6 @@
 
 #include "ldb_kv.h"
 #include "ldb_private.h"
-
-/*
-  add the special distinguishedName element
-*/
-static int msg_add_distinguished_name(struct ldb_message *msg)
-{
-       const char *dn_attr = "distinguishedName";
-       char *dn = NULL;
-
-       if (ldb_msg_find_element(msg, dn_attr)) {
-               /*
-                * This should not happen, but this is
-                * existing behaviour...
-                */
-               return LDB_SUCCESS;
-       }
-
-       dn = ldb_dn_alloc_linearized(msg, msg->dn);
-       if (dn == NULL) {
-               return LDB_ERR_OPERATIONS_ERROR;
-       }
-
-       return ldb_msg_add_steal_string(msg, dn_attr, dn);
-}
-
 /*
   search the database for a single simple dn.
   return LDB_ERR_NO_SUCH_OBJECT on record-not-found
@@ -327,124 +302,7 @@ int ldb_kv_filter_attrs(struct ldb_context *ldb,
                        const char *const *attrs,
                        struct ldb_message *filtered_msg)
 {
-       unsigned int i;
-       bool keep_all = false;
-       bool add_dn = false;
-       uint32_t num_elements;
-       uint32_t elements_size;
-
-       if (attrs) {
-               /* check for special attrs */
-               for (i = 0; attrs[i]; i++) {
-                       int cmp = strcmp(attrs[i], "*");
-                       if (cmp == 0) {
-                               keep_all = true;
-                               break;
-                       }
-                       cmp = ldb_attr_cmp(attrs[i], "distinguishedName");
-                       if (cmp == 0) {
-                               add_dn = true;
-                       }
-               }
-       } else {
-               keep_all = true;
-       }
-
-       if (keep_all) {
-               add_dn = true;
-               elements_size = msg->num_elements + 1;
-
-       /* Shortcuts for the simple cases */
-       } else if (add_dn && i == 1) {
-               if (msg_add_distinguished_name(filtered_msg) != 0) {
-                       goto failed;
-               }
-               return 0;
-       } else if (i == 0) {
-               return 0;
-
-       /* Otherwise we are copying at most as many element as we have attributes */
-       } else {
-               elements_size = i;
-       }
-
-       filtered_msg->elements = talloc_array(filtered_msg,
-                                             struct ldb_message_element,
-                                             elements_size);
-       if (filtered_msg->elements == NULL) goto failed;
-
-       num_elements = 0;
-
-       for (i = 0; i < msg->num_elements; i++) {
-               struct ldb_message_element *el = &msg->elements[i];
-               struct ldb_message_element *el2 = &filtered_msg->elements[num_elements];
-               unsigned int j;
-
-               if (keep_all == false) {
-                       bool found = false;
-                       for (j = 0; attrs[j]; j++) {
-                               int cmp = ldb_attr_cmp(el->name, attrs[j]);
-                               if (cmp == 0) {
-                                       found = true;
-                                       break;
-                               }
-                       }
-                       if (found == false) {
-                               continue;
-                       }
-               }
-               *el2 = *el;
-               el2->name = talloc_strdup(filtered_msg->elements,
-                                         el->name);
-               if (el2->name == NULL) {
-                       goto failed;
-               }
-               el2->values = talloc_array(filtered_msg->elements,
-                                          struct ldb_val, el->num_values);
-               if (el2->values == NULL) {
-                       goto failed;
-               }
-               for (j=0;j<el->num_values;j++) {
-                       el2->values[j] = ldb_val_dup(el2->values, &el->values[j]);
-                       if (el2->values[j].data == NULL && el->values[j].length != 0) {
-                               goto failed;
-                       }
-               }
-               num_elements++;
-
-               /* Pidginhole principle: we can't have more elements
-                * than the number of attributes if they are unique in
-                * the DB */
-               if (num_elements > elements_size) {
-                       goto failed;
-               }
-       }
-
-       filtered_msg->num_elements = num_elements;
-
-       if (add_dn) {
-               if (msg_add_distinguished_name(filtered_msg) != 0) {
-                       goto failed;
-               }
-       }
-
-       if (filtered_msg->num_elements > 0) {
-               filtered_msg->elements
-                       = talloc_realloc(filtered_msg,
-                                        filtered_msg->elements,
-                                        struct ldb_message_element,
-                                        filtered_msg->num_elements);
-               if (filtered_msg->elements == NULL) {
-                       goto failed;
-               }
-       } else {
-               talloc_free(filtered_msg->elements);
-               filtered_msg->elements = NULL;
-       }
-
-       return 0;
-failed:
-       return -1;
+       return ldb_filter_attrs(ldb, msg, attrs, filtered_msg);
 }
 
 /*

diff --git a/source4/torture/ldb/ldb.c b/source4/torture/ldb/ldb.c
index 780be179a59398131c22037b1712e9662038e0af..dfdce73d64f7954b369d4f3afc12e3fad1ddd973 100644 (file)
--- a/source4/torture/ldb/ldb.c
+++ b/source4/torture/ldb/ldb.c
@@ -1262,16 +1262,17 @@ static bool torture_ldb_pack_format_perf(struct torture_context *torture)
        return true;
 }
 
-static bool torture_ldb_unpack_only_attr_list(struct torture_context *torture)
+static bool torture_ldb_unpack_and_filter(struct torture_context *torture)
 {
        TALLOC_CTX *mem_ctx = talloc_new(torture);
        struct ldb_context *ldb;
        struct ldb_val data = data_blob_const(dda1d01d_bin, sizeof(dda1d01d_bin));
+       struct ldb_message *unpack_msg = ldb_msg_new(mem_ctx);
        struct ldb_message *msg = ldb_msg_new(mem_ctx);
-       const char *lookup_names[] = {"instanceType", "nonexistant", "whenChanged",
-                                     "objectClass", "uSNCreated",
-                                     "showInAdvancedViewOnly", "name", "cnNotHere"};
-       unsigned int nb_elements_in_db;
+       const char *lookup_names[] = {"instanceType", "nonexistant",
+                                     "whenChanged", "objectClass",
+                                     "uSNCreated", "showInAdvancedViewOnly",
+                                     "name", "cnNotHere", NULL};
        const char *ldif_text;
        struct ldb_ldif ldif;
 
@@ -1281,13 +1282,19 @@ static bool torture_ldb_unpack_only_attr_list(struct torture_context *torture)
                       "Failed to init samba");
 
        torture_assert_int_equal(torture,
-                                ldb_unpack_data_only_attr_list(ldb, &data, msg,
-                                                         lookup_names, ARRAY_SIZE(lookup_names),
-                                                         &nb_elements_in_db), 0,
-                                "ldb_unpack_data_only_attr_list failed");
-       torture_assert_int_equal(torture, nb_elements_in_db, 13,
+                                ldb_unpack_data(ldb, &data, unpack_msg),
+                                0, "ldb_unpack_data failed");
+
+       torture_assert_int_equal(torture, unpack_msg->num_elements, 13,
                                 "Got wrong count of elements");
 
+       msg->dn = talloc_steal(msg, unpack_msg->dn);
+
+       torture_assert_int_equal(torture,
+                                ldb_filter_attrs(ldb, unpack_msg,
+                                                 lookup_names, msg),
+                                0, "ldb_kv_filter_attrs failed");
+
        /* Compare data in binary form */
        torture_assert_int_equal(torture, msg->num_elements, 6,
                                 "Got wrong number of parsed elements");
@@ -1394,8 +1401,8 @@ struct torture_suite *torture_ldb(TALLOC_CTX *mem_ctx)
                                      torture_ldb_unpack_flags);
        torture_suite_add_simple_test(suite, "parse-ldif",
                                      torture_ldb_parse_ldif);
-       torture_suite_add_simple_test(suite, "unpack-data-only-attr-list",
-                                     torture_ldb_unpack_only_attr_list);
+       torture_suite_add_simple_test(suite, "unpack-and-filter",
+                                     torture_ldb_unpack_and_filter);
        torture_suite_add_simple_test(suite, "pack-format-perf",
                                      torture_ldb_pack_format_perf);