From: Vsevolod Stakhov Date: Sun, 10 Apr 2022 11:51:49 +0000 (+0100) Subject: [Rework] Try to fix the mess with types & flags X-Git-Tag: 3.3~293^2~37 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b36bbbee87ce7757633b9d945f6bb4b194dbbb1f;p=thirdparty%2Frspamd.git [Rework] Try to fix the mess with types & flags --- diff --git a/src/libserver/symcache/symcache_c.cxx b/src/libserver/symcache/symcache_c.cxx index 7255f9d10c..7c204b04e6 100644 --- a/src/libserver/symcache/symcache_c.cxx +++ b/src/libserver/symcache/symcache_c.cxx @@ -46,3 +46,11 @@ rspamd_symcache_init (struct rspamd_symcache *cache) return real_cache->init(); } + +void +rspamd_symcache_save (struct rspamd_symcache *cache) +{ + auto *real_cache = C_API_SYMCACHE(cache); + + real_cache->save_items(); +} diff --git a/src/libserver/symcache/symcache_impl.cxx b/src/libserver/symcache/symcache_impl.cxx index aadd53b8f2..0cae798737 100644 --- a/src/libserver/symcache/symcache_impl.cxx +++ b/src/libserver/symcache/symcache_impl.cxx @@ -17,6 +17,7 @@ #include "symcache_internal.hxx" #include "unix-std.h" #include "libutil/cxx/locked_file.hxx" +#include "fmt/core.h" #include @@ -223,7 +224,7 @@ auto symcache::load_items() -> bool } } - if (item->is_virtual() && !(item->type & SYMBOL_TYPE_GHOST)) { + if (item->is_virtual() && !item->is_ghost()) { const auto &parent = item->get_parent(*this); if (parent) { @@ -613,29 +614,11 @@ auto cache_item::process_deps(const symcache &cache) -> void */ auto ok_dep = false; - if (is_filter()) { - if (dit->is_filter()) { - ok_dep = true; - } - else if (dit->type & SYMBOL_TYPE_PREFILTER) { - ok_dep = true; - } + if (dit->get_type() == type) { + ok_dep = true; } - else if (type & SYMBOL_TYPE_POSTFILTER) { - if (dit->type & SYMBOL_TYPE_PREFILTER) { - ok_dep = true; - } - } - else if (type & SYMBOL_TYPE_IDEMPOTENT) { - if (dit->type & (SYMBOL_TYPE_PREFILTER|SYMBOL_TYPE_POSTFILTER)) { - ok_dep = true; - } - } - else if (type & SYMBOL_TYPE_PREFILTER) { - if (priority < dit->priority) { - /* Also OK */ - ok_dep = true; - } + else if (type < dit->get_type()) { + ok_dep = true; } if (!ok_dep) { @@ -683,4 +666,81 @@ auto virtual_item::get_parent(const symcache &cache) const -> const cache_item * return cache.get_item_by_id(parent_id, false); } +auto item_type_from_c(enum rspamd_symbol_type type) -> tl::expected, std::string> +{ + constexpr const auto trivial_types = SYMBOL_TYPE_CONNFILTER|SYMBOL_TYPE_PREFILTER + |SYMBOL_TYPE_POSTFILTER|SYMBOL_TYPE_IDEMPOTENT + |SYMBOL_TYPE_COMPOSITE|SYMBOL_TYPE_CLASSIFIER + |SYMBOL_TYPE_VIRTUAL; + + constexpr auto all_but_one_ty = [&](int type, int exclude_bit) -> auto { + return type & (trivial_types & ~exclude_bit); + }; + + if (type & trivial_types) { + auto check_trivial = [&](auto flag, symcache_item_type ty) -> tl::expected, std::string> { + if (all_but_one_ty(type, flag)) { + return tl::make_unexpected(fmt::format("invalid flags for a symbol: {}", type)); + } + + return std::make_pair(ty, type & ~flag); + }; + if (type & SYMBOL_TYPE_CONNFILTER) { + return check_trivial(SYMBOL_TYPE_CONNFILTER, symcache_item_type::CONNFILTER); + } + else if (type & SYMBOL_TYPE_PREFILTER) { + return check_trivial(SYMBOL_TYPE_PREFILTER, symcache_item_type::PREFILTER); + } + else if (type & SYMBOL_TYPE_POSTFILTER) { + return check_trivial(SYMBOL_TYPE_POSTFILTER, symcache_item_type::POSTFILTER); + } + else if (type & SYMBOL_TYPE_IDEMPOTENT) { + return check_trivial(SYMBOL_TYPE_IDEMPOTENT, symcache_item_type::IDEMPOTENT); + } + else if (type & SYMBOL_TYPE_COMPOSITE) { + return check_trivial(SYMBOL_TYPE_COMPOSITE, symcache_item_type::COMPOSITE); + } + else if (type & SYMBOL_TYPE_CLASSIFIER) { + return check_trivial(SYMBOL_TYPE_CLASSIFIER, symcache_item_type::CLASSIFIER); + } + else if (type & SYMBOL_TYPE_VIRTUAL) { + return check_trivial(SYMBOL_TYPE_VIRTUAL, symcache_item_type::VIRTUAL); + } + + return tl::make_unexpected(fmt::format("internal error: impossible flags combination", type)); + } + + /* Maybe check other flags combination here? */ + return std::make_pair(symcache_item_type::FILTER, type); +} + +bool operator<(symcache_item_type lhs, symcache_item_type rhs) +{ + auto ret = false; + switch(lhs) { + case symcache_item_type::CONNFILTER: + break; + case symcache_item_type::PREFILTER: + if (rhs == symcache_item_type::CONNFILTER) { + ret = true; + } + break; + case symcache_item_type::FILTER: + if (rhs == symcache_item_type::CONNFILTER || rhs == symcache_item_type::PREFILTER) { + ret = true; + } + break; + case symcache_item_type::POSTFILTER: + if (rhs != symcache_item_type::IDEMPOTENT) { + ret = true; + } + break; + case symcache_item_type::IDEMPOTENT: + default: + break; + } + + return ret; +} + } \ No newline at end of file diff --git a/src/libserver/symcache/symcache_internal.hxx b/src/libserver/symcache/symcache_internal.hxx index 7dd664e5ca..62b8e8d999 100644 --- a/src/libserver/symcache/symcache_internal.hxx +++ b/src/libserver/symcache/symcache_internal.hxx @@ -32,7 +32,7 @@ #include #include #include "contrib/robin-hood/robin_hood.h" - +#include "contrib/expected/expected.hpp" #include "cfg_file.h" #include "lua/lua_common.h" @@ -90,6 +90,29 @@ using order_generation_ptr = std::shared_ptr; class symcache; +enum class symcache_item_type { + CONNFILTER, /* Executed on connection stage */ + PREFILTER, /* Executed before all filters */ + FILTER, /* Normal symbol with a callback */ + POSTFILTER, /* Executed after all filters */ + IDEMPOTENT, /* Executed after postfilters, cannot change results */ + CLASSIFIER, /* A virtual classifier symbol */ + COMPOSITE, /* A virtual composite symbol */ + VIRTUAL, /* A virtual symbol... */ +}; + +/* + * Compare item types: earlier stages symbols are > than later stages symbols + * Order for virtual stuff is not defined. + */ +bool operator < (symcache_item_type lhs, symcache_item_type rhs); +/** + * This is a public helper to convert a legacy C type to a more static type + * @param type input type as a C enum + * @return pair of type safe symcache_item_type + the remaining flags or an error + */ +auto item_type_from_c(enum rspamd_symbol_type type) -> tl::expected, std::string>; + struct item_condition { private: lua_State *L; @@ -151,7 +174,8 @@ struct cache_item : std::enable_shared_from_this { std::uint64_t last_count = 0; std::string symbol; std::string_view type_descr; - int type; + symcache_item_type type; + int flags; /* Callback data */ std::variant specific; @@ -196,16 +220,16 @@ public: auto process_deps(const symcache &cache) -> void; auto is_virtual() const -> bool { return std::holds_alternative(specific); } auto is_filter() const -> bool { - static constexpr const auto forbidden_flags = SYMBOL_TYPE_PREFILTER| - SYMBOL_TYPE_COMPOSITE| - SYMBOL_TYPE_CONNFILTER| - SYMBOL_TYPE_POSTFILTER| - SYMBOL_TYPE_IDEMPOTENT; - return std::holds_alternative(specific) && - (type & forbidden_flags) == 0; + (type == symcache_item_type::FILTER); + } + auto is_ghost() const -> bool { + return flags & SYMBOL_TYPE_GHOST; } auto get_parent(const symcache &cache) const -> const cache_item *; + auto get_type() const -> auto { + return type; + } auto add_condition(lua_State *L, int cbref) -> bool { if (!is_virtual()) { auto &normal = std::get(specific); @@ -271,7 +295,6 @@ private: private: /* Internal methods */ auto load_items() -> bool; - auto save_items() const -> bool; auto resort() -> void; /* Helper for g_hash_table_foreach */ static auto metric_connect_cb(void *k, void *v, void *ud) -> void; @@ -297,6 +320,12 @@ public: } } + /** + * Saves items on disk (if possible) + * @return + */ + auto save_items() const -> bool; + /** * Get an item by ID * @param id @@ -342,9 +371,20 @@ public: return cfg->checksum; } + /** + * Helper to return a memory pool associated with the cache + * @return + */ auto get_pool() const { return static_pool; } + + auto add_symbol_with_callback(std::string_view name, + int priority, + symbol_func_t func, + void *user_data, + enum rspamd_symbol_type type) -> int; + auto add_virtual_symbol() -> int; }; /*