ldb: move ldb_kv's filter into pack code
authorAndrew Bartlett <abartlet@samba.org>
Tue, 14 May 2019 23:04:42 +0000 (11:04 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Wed, 15 May 2019 04:03:37 +0000 (04:03 +0000)
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>
lib/ldb/common/ldb_pack.c
lib/ldb/include/ldb_module.h
lib/ldb/ldb_key_value/ldb_kv_search.c
source4/torture/ldb/ldb.c

index 60242e1..24c5065 100644 (file)
@@ -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;
+}
index bf4879d..8fa5060 100644 (file)
@@ -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
  *
index 8340368..7a1567b 100644 (file)
 
 #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);
 }
 
 /*
index 780be17..dfdce73 100644 (file)
@@ -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);