commit b36bbbe: [Rework] Try to fix the mess with types & flags

Vsevolod Stakhov vsevolod at rspamd.com
Sat Apr 30 19:21:23 UTC 2022


Author: Vsevolod Stakhov
Date: 2022-04-10 12:51:49 +0100
URL: https://github.com/rspamd/rspamd/commit/b36bbbee87ce7757633b9d945f6bb4b194dbbb1f

[Rework] Try to fix the mess with types & flags

---
 src/libserver/symcache/symcache_c.cxx        |   8 ++
 src/libserver/symcache/symcache_impl.cxx     | 106 +++++++++++++++++++++------
 src/libserver/symcache/symcache_internal.hxx |  60 ++++++++++++---
 3 files changed, 141 insertions(+), 33 deletions(-)

diff --git a/src/libserver/symcache/symcache_c.cxx b/src/libserver/symcache/symcache_c.cxx
index 7255f9d10..7c204b04e 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 aadd53b8f..0cae79873 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 <cmath>
 
@@ -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::pair<symcache_item_type, int>, 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::pair<symcache_item_type, int>, 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 7dd664e5c..62b8e8d99 100644
--- a/src/libserver/symcache/symcache_internal.hxx
+++ b/src/libserver/symcache/symcache_internal.hxx
@@ -32,7 +32,7 @@
 #include <memory>
 #include <variant>
 #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<order_generation>;
 
 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::pair<symcache_item_type, int>, std::string>;
+
 struct item_condition {
 private:
 	lua_State *L;
@@ -151,7 +174,8 @@ struct cache_item : std::enable_shared_from_this<cache_item> {
 	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<normal_item, virtual_item> specific;
@@ -196,16 +220,16 @@ public:
 	auto process_deps(const symcache &cache) -> void;
 	auto is_virtual() const -> bool { return std::holds_alternative<virtual_item>(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<normal_item>(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<normal_item>(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;
 };
 
 /*


More information about the Commits mailing list