dsdb/mod/extended_dn_out: use faster removal filters
authorDouglas Bagnall <douglas.bagnall@catalyst.net.nz>
Thu, 11 Apr 2019 01:14:24 +0000 (13:14 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Fri, 10 May 2019 01:15:18 +0000 (01:15 +0000)
When filtering out multiple elements, we end up memmove()ing the same
elements many times over. It is simpler to not do that by keeping track
of how many elements we are keeping.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
source4/dsdb/samdb/ldb_modules/extended_dn_out.c

index 4228a2ab69df2576aed464eff61e44317e9ceeb3..7fc0a6cffd8cf5cdf76aa5df4c2a5d6070c10f24 100644 (file)
@@ -396,7 +396,7 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
        struct ldb_control *control;
        struct dsdb_openldap_dereference_result_control *dereference_control = NULL;
        int ret;
-       unsigned int i, j;
+       unsigned int i, j, k;
        struct ldb_message *msg;
        struct extended_dn_out_private *p;
        struct ldb_context *ldb;
@@ -521,11 +521,11 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
                        make_extended_dn = (strcmp(attribute->syntax->ldap_oid, DSDB_SYNTAX_OR_NAME) != 0);
                }
 
-               for (j = 0; j < msg->elements[i].num_values; j++) {
+               for (k = 0, j = 0; j < msg->elements[i].num_values; j++) {
                        const char *dn_str;
                        struct ldb_dn *dn;
                        struct dsdb_dn *dsdb_dn = NULL;
-                       struct ldb_val *plain_dn = &msg->elements[i].values[j];         
+                       struct ldb_val *plain_dn = &msg->elements[i].values[j];
                        bool is_deleted_objects = false;
 
                        /* this is a fast method for detecting deleted
@@ -534,11 +534,7 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
                        if (dsdb_dn_is_deleted_val(plain_dn) && !have_reveal_control) {
                                /* it's a deleted linked attribute,
                                  and we don't have the reveal control */
-                               memmove(&msg->elements[i].values[j],
-                                       &msg->elements[i].values[j+1],
-                                       (msg->elements[i].num_values-(j+1))*sizeof(struct ldb_val));
-                               msg->elements[i].num_values--;
-                               j--;
+                               /* we won't keep this one, so not incrementing k */
                                continue;
                        }
 
@@ -546,8 +542,8 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
                        dsdb_dn = dsdb_dn_parse_trusted(msg, ldb, plain_dn, attribute->syntax->ldap_oid);
 
                        if (!dsdb_dn) {
-                               ldb_asprintf_errstring(ldb, 
-                                                      "could not parse %.*s in %s on %s as a %s DN", 
+                               ldb_asprintf_errstring(ldb,
+                                                      "could not parse %.*s in %s on %s as a %s DN",
                                                       (int)plain_dn->length, plain_dn->data,
                                                       msg->elements[i].name, ldb_dn_get_linearized(msg->dn),
                                                       attribute->syntax->ldap_oid);
@@ -574,7 +570,7 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
                                        return ldb_module_done(ac->req, NULL, NULL, ret);
                                }
                        }
-                       
+
                        /* If we are running in dereference mode (such
                         * as against OpenLDAP) then the DN in the msg
                         * above does not contain the extended values,
@@ -611,11 +607,7 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
                                        /* we show these with REVEAL
                                           to allow dbcheck to find and
                                           cleanup these orphaned links */
-                                       memmove(&msg->elements[i].values[j],
-                                               &msg->elements[i].values[j+1],
-                                               (msg->elements[i].num_values-(j+1))*sizeof(struct ldb_val));
-                                       msg->elements[i].num_values--;
-                                       j--;
+                                       /* we won't keep this one, so not incrementing k */
                                        continue;
                                }
                        }
@@ -639,23 +631,27 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
                                dn_str = dsdb_dn_get_extended_linearized(msg->elements[i].values,
                                                                         dsdb_dn, ac->extended_type);
                        } else {
-                               dn_str = dsdb_dn_get_linearized(msg->elements[i].values, 
+                               dn_str = dsdb_dn_get_linearized(msg->elements[i].values,
                                                                dsdb_dn);
                        }
-                       
+
                        if (!dn_str) {
                                ldb_oom(ldb);
                                talloc_free(dsdb_dn);
                                return ldb_module_done(ac->req, NULL, NULL, LDB_ERR_OPERATIONS_ERROR);
                        }
-                       msg->elements[i].values[j] = data_blob_string_const(dn_str);
+                       msg->elements[i].values[k] = data_blob_string_const(dn_str);
                        talloc_free(dsdb_dn);
+                       k++;
                }
-               if (msg->elements[i].num_values == 0) {
+
+               if (k == 0) {
                        /* we've deleted all of the values from this
                         * element - remove the element */
                        ldb_msg_remove_element(msg, &msg->elements[i]);
                        i--;
+               } else {
+                       msg->elements[i].num_values = k;
                }
        }
        return ldb_module_send_entry(ac->req, msg, ares->controls);