diff options
| -rw-r--r-- | lib/ldb/common/ldb_pack.c | 30 | ||||
| -rw-r--r-- | lib/ldb/include/ldb_module.h | 12 | ||||
| -rw-r--r-- | lib/ldb/ldb_key_value/ldb_kv.c | 11 | ||||
| -rw-r--r-- | lib/ldb/ldb_key_value/ldb_kv_cache.c | 6 | ||||
| -rw-r--r-- | lib/ldb/ldb_key_value/ldb_kv_index.c | 33 | ||||
| -rw-r--r-- | lib/ldb/ldb_key_value/ldb_kv_search.c | 74 | ||||
| -rw-r--r-- | source4/torture/ldb/ldb.c | 18 |
7 files changed, 52 insertions, 132 deletions
diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c index 448c577ae1b..60242e178f6 100644 --- a/lib/ldb/common/ldb_pack.c +++ b/lib/ldb/common/ldb_pack.c @@ -326,10 +326,8 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb, * In typical use, most values are single-valued. This makes * it quite expensive to allocate an array of ldb_val for each * of these, just to then hold the pointer to the data buffer - * (in the LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC we don't - * allocate the data). So with - * LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC we allocate this ahead - * of time and use it for the single values where possible. + * So with LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC we allocate this + * ahead of time and use it for the single values where possible. * (This is used the the normal search case, but not in the * index case because of caller requirements). */ @@ -405,16 +403,7 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb, } } element = &message->elements[nelem]; - if (flags & LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC) { - element->name = attr; - } else { - element->name = talloc_memdup(message->elements, attr, attr_len+1); - - if (element->name == NULL) { - errno = ENOMEM; - goto failed; - } - } + element->name = attr; element->flags = 0; if (remaining < (attr_len + 1)) { @@ -460,18 +449,7 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb, } element->values[j].length = len; - if (flags & LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC) { - element->values[j].data = p + 4; - } else { - element->values[j].data = talloc_size(element->values, len+1); - if (element->values[j].data == NULL) { - errno = ENOMEM; - goto failed; - } - memcpy(element->values[j].data, p + 4, - len); - element->values[j].data[len] = 0; - } + element->values[j].data = p + 4; remaining -= len; p += len+4+1; } diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h index b45142abe5c..bf4879d1ca5 100644 --- a/lib/ldb/include/ldb_module.h +++ b/lib/ldb/include/ldb_module.h @@ -528,20 +528,10 @@ int ldb_unpack_data(struct ldb_context *ldb, /* * 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. - * - * Flags allow control of allocation, so that if - * LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC is specified, then data in values are - * not allocated, instead they point into the supplier constant buffer. - * * If LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC is specified, then values * array are not allocated individually (for single-valued * attributes), instead they point into a single buffer per message. * - * LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC is only valid when - * LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC is also specified. - * * Likewise if LDB_UNPACK_DATA_FLAG_NO_DN is specified, the DN is omitted. * * If LDB_UNPACK_DATA_FLAG_NO_ATTRS is specified, then no attributes @@ -556,7 +546,7 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb, unsigned int flags, unsigned int *nb_elements_in_db); -#define LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC 0x0001 +/* currently unused (was NO_DATA_ALLOC) 0x0001 */ #define LDB_UNPACK_DATA_FLAG_NO_DN 0x0002 #define LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC 0x0004 #define LDB_UNPACK_DATA_FLAG_NO_ATTRS 0x0008 diff --git a/lib/ldb/ldb_key_value/ldb_kv.c b/lib/ldb/ldb_key_value/ldb_kv.c index c0846ba1b6a..53c326d36b2 100644 --- a/lib/ldb/ldb_key_value/ldb_kv.c +++ b/lib/ldb/ldb_key_value/ldb_kv.c @@ -644,8 +644,7 @@ static int ldb_kv_delete_internal(struct ldb_module *module, struct ldb_dn *dn) /* in case any attribute of the message was indexed, we need to fetch the old record */ - ret = ldb_kv_search_dn1( - module, dn, msg, LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC); + ret = ldb_kv_search_dn1(module, dn, msg, 0); if (ret != LDB_SUCCESS) { /* not finding the old record is an error */ goto done; @@ -902,8 +901,7 @@ int ldb_kv_modify_internal(struct ldb_module *module, goto done; } - ret = ldb_kv_search_dn1( - module, msg->dn, msg2, LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC); + ret = ldb_kv_search_dn1(module, msg->dn, msg2, 0); if (ret != LDB_SUCCESS) { goto done; } @@ -1267,10 +1265,7 @@ static int ldb_kv_rename(struct ldb_kv_context *ctx) } /* we need to fetch the old record to re-add under the new name */ - ret = ldb_kv_search_dn1(module, - req->op.rename.olddn, - msg, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC); + ret = ldb_kv_search_dn1(module, req->op.rename.olddn, msg, 0); if (ret == LDB_ERR_INVALID_DN_SYNTAX) { ldb_asprintf_errstring(ldb_module_get_ctx(module), "Invalid Old DN: %s", diff --git a/lib/ldb/ldb_key_value/ldb_kv_cache.c b/lib/ldb/ldb_key_value/ldb_kv_cache.c index 091d62d4158..b14697c2a5e 100644 --- a/lib/ldb/ldb_key_value/ldb_kv_cache.c +++ b/lib/ldb/ldb_key_value/ldb_kv_cache.c @@ -127,8 +127,7 @@ static int ldb_kv_attributes_load(struct ldb_module *module) r = ldb_kv_search_dn1(module, dn, attrs_msg, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC | - LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC | + LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC | LDB_UNPACK_DATA_FLAG_NO_DN); talloc_free(dn); if (r != LDB_SUCCESS && r != LDB_ERR_NO_SUCH_OBJECT) { @@ -278,8 +277,7 @@ static int ldb_kv_index_load(struct ldb_module *module, r = ldb_kv_search_dn1(module, indexlist_dn, ldb_kv->cache->indexlist, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC | - LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC | + LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC | LDB_UNPACK_DATA_FLAG_NO_DN); TALLOC_FREE(indexlist_dn); diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c index 9f0de7d260e..0bedd2df517 100644 --- a/lib/ldb/ldb_key_value/ldb_kv_index.c +++ b/lib/ldb/ldb_key_value/ldb_kv_index.c @@ -405,7 +405,6 @@ normal_index: ret = ldb_kv_search_dn1(module, dn, msg, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC | LDB_UNPACK_DATA_FLAG_NO_DN | /* * The entry point ldb_kv_search_indexed is @@ -428,9 +427,8 @@ normal_index: /* * we avoid copying the strings by stealing the list. We have - * to steal msg onto el->values (which looks odd) because we - * asked for the memory to be allocated on msg, not on each - * value with LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC above + * to steal msg onto el->values (which looks odd) because + * the memory is allocated on msg, not on each value. */ if (ldb_kv->cache->GUID_index_attribute == NULL) { /* check indexing version number */ @@ -481,8 +479,7 @@ normal_index: } /* - * The actual data is on msg, due to - * LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC + * The actual data is on msg. */ talloc_steal(list->dn, msg); for (i = 0; i < list->count; i++) { @@ -1675,7 +1672,6 @@ static int traverse_range_index(struct ldb_kv_private *ldb_kv, ctx->error = ldb_unpack_data_only_attr_list_flags(ldb, &data, msg, NULL, 0, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC | LDB_UNPACK_DATA_FLAG_NO_DN, NULL); @@ -1694,9 +1690,8 @@ static int traverse_range_index(struct ldb_kv_private *ldb_kv, /* * we avoid copying the strings by stealing the list. We have - * to steal msg onto el->values (which looks odd) because we - * asked for the memory to be allocated on msg, not on each - * value with LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC above + * to steal msg onto el->values (which looks odd) because + * the memory is allocated on msg, not on each value. */ if (version != LDB_KV_GUID_INDEXING_VERSION) { /* This is quite likely during the DB startup @@ -1758,8 +1753,7 @@ static int traverse_range_index(struct ldb_kv_private *ldb_kv, } /* - * The actual data is on msg, due to - * LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC + * The actual data is on msg. */ talloc_steal(ctx->dn_list->dn, msg); for (i = 0; i < additional_length; i++) { @@ -2227,7 +2221,6 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv, ldb_kv, keys[i], msg, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC | LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC | /* * The entry point ldb_kv_search_indexed is @@ -3270,7 +3263,6 @@ static int re_key(struct ldb_kv_private *ldb_kv, (struct ldb_kv_reindex_context *)state; struct ldb_module *module = ctx->module; struct ldb_message *msg; - unsigned int nb_elements_in_db; int ret; struct ldb_val key2; bool is_record; @@ -3287,11 +3279,7 @@ static int re_key(struct ldb_kv_private *ldb_kv, return -1; } - ret = ldb_unpack_data_only_attr_list_flags(ldb, &val, - msg, - NULL, 0, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC, - &nb_elements_in_db); + ret = ldb_unpack_data(ldb, &val, msg); if (ret != 0) { ldb_debug(ldb, LDB_DEBUG_ERROR, "Invalid data for index %s\n", ldb_dn_get_linearized(msg->dn)); @@ -3353,7 +3341,6 @@ static int re_index(struct ldb_kv_private *ldb_kv, (struct ldb_kv_reindex_context *)state; struct ldb_module *module = ctx->module; struct ldb_message *msg; - unsigned int nb_elements_in_db; int ret; bool is_record; @@ -3369,11 +3356,7 @@ static int re_index(struct ldb_kv_private *ldb_kv, return -1; } - ret = ldb_unpack_data_only_attr_list_flags(ldb, &val, - msg, - NULL, 0, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC, - &nb_elements_in_db); + ret = ldb_unpack_data(ldb, &val, msg); if (ret != 0) { ldb_debug(ldb, LDB_DEBUG_ERROR, "Invalid data for index %s\n", ldb_dn_get_linearized(msg->dn)); diff --git a/lib/ldb/ldb_key_value/ldb_kv_search.c b/lib/ldb/ldb_key_value/ldb_kv_search.c index 7a6ae5668b0..2834279d86e 100644 --- a/lib/ldb/ldb_key_value/ldb_kv_search.c +++ b/lib/ldb/ldb_key_value/ldb_kv_search.c @@ -133,49 +133,45 @@ static int ldb_kv_parse_data_unpack(struct ldb_val key, void *private_data) { struct ldb_kv_parse_data_unpack_ctx *ctx = private_data; - unsigned int nb_elements_in_db; int ret; struct ldb_context *ldb = ldb_module_get_ctx(ctx->module); struct ldb_val data_parse = data; struct ldb_kv_private *ldb_kv = ctx->ldb_kv; - if ((ctx->unpack_flags & LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC)) { - if ((ldb_kv->kv_ops->options & LDB_KV_OPTION_STABLE_READ_LOCK) && - (ctx->unpack_flags & LDB_UNPACK_DATA_FLAG_READ_LOCKED) && - !ldb_kv->kv_ops->transaction_active(ldb_kv)) { - /* - * In the case where no transactions are active and - * we're in a read-lock, we can point directly into - * database memory. - * - * The database can't be changed underneath us and we - * will duplicate this data in the call to filter. - * - * This is seen in: - * - ldb_kv_index_filter - * - ldb_kv_search_and_return_base - */ - } else { - /* - * In every other case, if we got - * LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC we need at least - * do a memdup on the whole data buffer as that may - * change later and the caller needs a stable result. - * - * During transactions, pointers could change and in - * TDB, there just aren't the same guarantees. - */ - data_parse.data = talloc_memdup(ctx->msg, - data.data, - data.length); - if (data_parse.data == NULL) { - ldb_debug(ldb, LDB_DEBUG_ERROR, - "Unable to allocate data(%d) for %*.*s\n", - (int)data.length, - (int)key.length, (int)key.length, key.data); - return LDB_ERR_OPERATIONS_ERROR; - } + if ((ldb_kv->kv_ops->options & LDB_KV_OPTION_STABLE_READ_LOCK) && + (ctx->unpack_flags & LDB_UNPACK_DATA_FLAG_READ_LOCKED) && + !ldb_kv->kv_ops->transaction_active(ldb_kv)) { + /* + * In the case where no transactions are active and + * we're in a read-lock, we can point directly into + * database memory. + * + * The database can't be changed underneath us and we + * will duplicate this data in the call to filter. + * + * This is seen in: + * - ldb_kv_index_filter + * - ldb_kv_search_and_return_base + */ + } else { + /* + * In every other case, since unpack doesn't memdup, we need + * to at least do a memdup on the whole data buffer as that + * may change later and the caller needs a stable result. + * + * During transactions, pointers could change and in + * TDB, there just aren't the same guarantees. + */ + data_parse.data = talloc_memdup(ctx->msg, + data.data, + data.length); + if (data_parse.data == NULL) { + ldb_debug(ldb, LDB_DEBUG_ERROR, + "Unable to allocate data(%d) for %*.*s\n", + (int)data.length, + (int)key.length, (int)key.length, key.data); + return LDB_ERR_OPERATIONS_ERROR; } } @@ -183,7 +179,7 @@ static int ldb_kv_parse_data_unpack(struct ldb_val key, ctx->msg, NULL, 0, ctx->unpack_flags, - &nb_elements_in_db); + NULL); if (ret == -1) { if (data_parse.data != data.data) { talloc_free(data_parse.data); @@ -514,7 +510,6 @@ static int search_func(struct ldb_kv_private *ldb_kv, ret = ldb_unpack_data_only_attr_list_flags(ldb, &val, msg, NULL, 0, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC| LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC, &nb_elements_in_db); if (ret == -1) { @@ -605,7 +600,6 @@ static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv, ret = ldb_kv_search_dn1(ctx->module, ctx->base, msg, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC | LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC | LDB_UNPACK_DATA_FLAG_READ_LOCKED); diff --git a/source4/torture/ldb/ldb.c b/source4/torture/ldb/ldb.c index dcbe919d5bf..261113841a5 100644 --- a/source4/torture/ldb/ldb.c +++ b/source4/torture/ldb/ldb.c @@ -1121,24 +1121,6 @@ static bool torture_ldb_unpack_flags(struct torture_context *torture) ldb_unpack_data_only_attr_list_flags(ldb, &data, msg, NULL, 0, - LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC, - &nb_elements_in_db), - 0, - "ldb_unpack_data failed"); - - ldif.changetype = LDB_CHANGETYPE_NONE; - ldif.msg = msg; - ldif_text = ldb_ldif_write_string(ldb, mem_ctx, &ldif); - - torture_assert_int_equal(torture, - strcmp(ldif_text, dda1d01d_ldif), 0, - "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_DATA_ALLOC| LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC, &nb_elements_in_db), 0, |