From 30addc1a5e322fa64c1b6106334b71becb96abb9 Mon Sep 17 00:00:00 2001 From: kannibalox Date: Mon, 10 Jun 2024 13:09:29 -0400 Subject: [PATCH] Use regular iterators intead of bucket-local ones Trying to manually predict which bucket a hashed key will land in does not appear to be well-defined behavior. Fixes #1285 --- src/command_dynamic.cc | 6 ++--- src/rpc/object_storage.cc | 49 ++++++++++++++++++--------------------- src/rpc/object_storage.h | 9 +++---- 3 files changed, 29 insertions(+), 35 deletions(-) diff --git a/src/command_dynamic.cc b/src/command_dynamic.cc index 0a1f8e7b..9ce015da 100644 --- a/src/command_dynamic.cc +++ b/src/command_dynamic.cc @@ -146,7 +146,7 @@ system_method_insert_object(const torrent::Object::list_type& args, int flags) { const std::string& rawKey = (itrArgs++)->as_string(); if (rawKey.empty() || - control->object_storage()->find_local(torrent::raw_string::from_string(rawKey)) != control->object_storage()->end(0) || + control->object_storage()->find_local(torrent::raw_string::from_string(rawKey)) != control->object_storage()->end() || rpc::commands.has(rawKey) || rpc::commands.has(rawKey + ".set")) throw torrent::input_error("Invalid key."); @@ -355,10 +355,10 @@ system_method_set_function(const torrent::Object::list_type& args) { if (args.empty()) throw torrent::input_error("Invalid argument count."); - rpc::object_storage::local_iterator itr = + rpc::object_storage::iterator itr = control->object_storage()->find_local(torrent::raw_string::from_string(args.front().as_string())); - if (itr == control->object_storage()->end(0) || itr->second.flags & rpc::object_storage::flag_constant) + if (itr == control->object_storage()->end() || itr->second.flags & rpc::object_storage::flag_constant) throw torrent::input_error("Command is not modifiable."); return control->object_storage()->set_str_function(args.front().as_string(), diff --git a/src/rpc/object_storage.cc b/src/rpc/object_storage.cc index 9f9ce952..dfaf6c6a 100644 --- a/src/rpc/object_storage.cc +++ b/src/rpc/object_storage.cc @@ -61,22 +61,19 @@ const unsigned int object_storage::flag_rlookup; const size_t object_storage::key_size; -object_storage::local_iterator +object_storage::iterator object_storage::find_local(const torrent::raw_string& key) { - std::size_t n = hash_fixed_key_type::hash(key.data(), key.size()) % bucket_count(); + fixed_key_type<64> k; + k.set_c_str(key.data()); - for (local_iterator itr = begin(n), last = end(n); itr != last; itr++) - if (itr->first.size() == key.size() && std::memcmp(itr->first.data(), key.data(), key.size()) == 0) - return itr; - - return end(bucket_count()); + return find(k); } -object_storage::local_iterator +object_storage::iterator object_storage::find_local_const(const torrent::raw_string& key, unsigned int type) { - local_iterator itr = find_local(key); + iterator itr = find_local(key); - if (itr == end(bucket_count())) + if (itr == end()) throw torrent::input_error("Key not found."); if ((type != 0 && (itr->second.flags & mask_type) != type)) @@ -85,11 +82,11 @@ object_storage::find_local_const(const torrent::raw_string& key, unsigned int ty return itr; } -object_storage::local_iterator +object_storage::iterator object_storage::find_local_mutable(const torrent::raw_string& key, unsigned int type) { - local_iterator itr = find_local(key); + iterator itr = find_local(key); - if (itr == end(bucket_count())) + if (itr == end()) throw torrent::input_error("Key not found."); if ((type != 0 && (itr->second.flags & mask_type) != type) || @@ -138,63 +135,63 @@ object_storage::insert(const char* key_data, uint32_t key_size, const torrent::O bool object_storage::has_flag(const torrent::raw_string& key, unsigned int flag) { - local_iterator itr = find_local_const(key); + iterator itr = find_local_const(key); return itr->second.flags & flag; } void object_storage::enable_flag(const torrent::raw_string& key, unsigned int flag) { - local_iterator itr = find_local_mutable(key); + iterator itr = find_local_mutable(key); itr->second.flags |= (flag & (flag_constant)); } const torrent::Object& object_storage::get(const torrent::raw_string& key) { - local_iterator itr = find_local_const(key); + iterator itr = find_local_const(key); return itr->second.object; } const torrent::Object& object_storage::set_bool(const torrent::raw_string& key, int64_t object) { - local_iterator itr = find_local_mutable(key, flag_bool_type); + iterator itr = find_local_mutable(key, flag_bool_type); return itr->second.object = !!object; } const torrent::Object& object_storage::set_value(const torrent::raw_string& key, int64_t object) { - local_iterator itr = find_local_mutable(key, flag_value_type); + iterator itr = find_local_mutable(key, flag_value_type); return itr->second.object = object; } const torrent::Object& object_storage::set_string(const torrent::raw_string& key, const std::string& object) { - local_iterator itr = find_local_mutable(key, flag_string_type); + iterator itr = find_local_mutable(key, flag_string_type); return itr->second.object = object; } const torrent::Object& object_storage::set_list(const torrent::raw_string& key, const torrent::Object::list_type& object) { - local_iterator itr = find_local_mutable(key, flag_list_type); + iterator itr = find_local_mutable(key, flag_list_type); return itr->second.object = torrent::Object::create_list_range(object.begin(), object.end()); } void object_storage::list_push_back(const torrent::raw_string& key, const torrent::Object& object) { - local_iterator itr = find_local_mutable(key, flag_list_type); + iterator itr = find_local_mutable(key, flag_list_type); itr->second.object.as_list().push_back(object); } const torrent::Object& object_storage::set_function(const torrent::raw_string& key, const std::string& object) { - local_iterator itr = find_local_mutable(key, flag_function_type); + iterator itr = find_local_mutable(key, flag_function_type); return itr->second.object = object; } torrent::Object object_storage::call_function(const torrent::raw_string& key, target_type target, const torrent::Object& object) { - local_iterator itr = find_local_const(key); + iterator itr = find_local_const(key); switch (itr->second.flags & mask_type) { case flag_function_type: @@ -207,13 +204,13 @@ object_storage::call_function(const torrent::raw_string& key, target_type target bool object_storage::has_multi_key(const torrent::raw_string& key, const std::string& cmd_key) { - local_iterator itr = find_local_const(key, flag_multi_type); + iterator itr = find_local_const(key, flag_multi_type); return itr->second.object.has_key(cmd_key); } void object_storage::erase_multi_key(const torrent::raw_string& key, const std::string& cmd_key) { - local_iterator itr = find_local_mutable(key, flag_multi_type); + iterator itr = find_local_mutable(key, flag_multi_type); itr->second.object.erase_key(cmd_key); @@ -238,7 +235,7 @@ object_storage::set_multi_key_obj(const torrent::raw_string& key, const std::str if (!object.is_string() && !object.is_dict_key() && !object.is_list()) throw torrent::input_error("Object is wrong type."); - local_iterator itr = find_local_mutable(key, flag_multi_type); + iterator itr = find_local_mutable(key, flag_multi_type); if (itr->second.flags & flag_rlookup) { rlookup_iterator r_itr = m_rlookup.find(cmd_key); diff --git a/src/rpc/object_storage.h b/src/rpc/object_storage.h index cabe7b3d..d5321177 100644 --- a/src/rpc/object_storage.h +++ b/src/rpc/object_storage.h @@ -79,9 +79,6 @@ public: using base_type::size; using base_type::empty; using base_type::key_eq; - using base_type::bucket; - using base_type::bucket_count; - using base_type::max_bucket_count; using base_type::load_factor; // Verify rlookup is static / const. @@ -107,9 +104,9 @@ public: static const size_t key_size = key_type::max_size; - local_iterator find_local(const torrent::raw_string& key); - local_iterator find_local_const(const torrent::raw_string& key, unsigned int type = 0); - local_iterator find_local_mutable(const torrent::raw_string& key, unsigned int type = 0); + iterator find_local(const torrent::raw_string& key); + iterator find_local_const(const torrent::raw_string& key, unsigned int type = 0); + iterator find_local_mutable(const torrent::raw_string& key, unsigned int type = 0); iterator insert(const char* key_data, uint32_t key_size, const torrent::Object& object, unsigned int flags); iterator insert_c_str(const char* key, const torrent::Object& object, unsigned int flags) { return insert(key, std::strlen(key), object, flags); }