update: bump libs version and apply C++ best practices in engine

cmake/modules/falcosecurity-libs.cmake

@@ -24,8 +24,8 @@ else()
   # default below In case you want to test against another falcosecurity/libs version just pass the variable - ie., `cmake
   # -DFALCOSECURITY_LIBS_VERSION=dev ..`
   if(NOT FALCOSECURITY_LIBS_VERSION)
-    set(FALCOSECURITY_LIBS_VERSION "b19f87e8aee663e4987a3db54570725e071ed105")
-    set(FALCOSECURITY_LIBS_CHECKSUM "SHA256=fd5888588796bf52848cf75434784770e61d9a79f344bf571fe495b61e92ddd3")
+    set(FALCOSECURITY_LIBS_VERSION "d6b75db133602dee81b4408902f2510275feae57")
+    set(FALCOSECURITY_LIBS_CHECKSUM "SHA256=396f784924d2916431ecfbfcd3732cc328f29c543c7cf5a917053f634ef6f7d4")
   endif()
 
   # cd /path/to/build && cmake /path/to/source

tests/engine/test_filter_evttype_resolver.cpp

@@ -38,8 +38,7 @@ string to_string(set<uint16_t> s)
 void compare_evttypes(ast::expr* f, set<uint16_t> &expected)
 {
     set<uint16_t> actual;
-    filter_evttype_resolver resolver;
-    resolver.evttypes(f, actual);
+    filter_evttype_resolver().evttypes(f, actual);
     for(auto &etype : expected)
     {
         REQUIRE(actual.find(etype) != actual.end());
@@ -52,13 +51,11 @@ void compare_evttypes(ast::expr* f, set<uint16_t> &expected)
 
 ast::expr* compile(const string &fltstr)
 {
-    libsinsp::filter::parser p(fltstr);
-    return p.parse();
+    return libsinsp::filter::parser(fltstr).parse();
 }
 
 TEST_CASE("Should find event types from filter", "[rule_loader]")
 {
-    filter_evttype_resolver resolver;
     set<uint16_t> openat_only{
 		PPME_SYSCALL_OPENAT_2_E, PPME_SYSCALL_OPENAT_2_X };
 

userspace/engine/filter_evttype_resolver.cpp

@@ -27,7 +27,7 @@ static bool is_evttype_operator(const string& op)
 	return op == "==" || op == "=" || op == "!=" || op == "in";
 }
 
-void filter_evttype_resolver::inversion(set<uint16_t>& types)
+void filter_evttype_resolver::visitor::inversion(set<uint16_t>& types)
 {
 	set<uint16_t> all_types;
 	evttypes("", all_types);
@@ -41,7 +41,7 @@ void filter_evttype_resolver::inversion(set<uint16_t>& types)
 	}
 }
 
-void filter_evttype_resolver::evttypes(string evtname, set<uint16_t>& out)
+void filter_evttype_resolver::visitor::evttypes(string evtname, set<uint16_t>& out)
 {
 	// Fill in from 2 to PPM_EVENT_MAX-1. 0 and 1 are excluded as
 	// those are PPM_GENERIC_E/PPME_GENERIC_X
@@ -58,26 +58,31 @@ void filter_evttype_resolver::evttypes(string evtname, set<uint16_t>& out)
 	}
 }
 
-void filter_evttype_resolver::evttypes(ast::expr* filter, set<uint16_t>& out)
+void filter_evttype_resolver::evttypes(
+	ast::expr* filter,
+	set<uint16_t>& out) const
 {
-	m_expect_value = false;
-	m_last_node_evttypes.clear();
-	filter->accept(this);
-	out.insert(m_last_node_evttypes.begin(), m_last_node_evttypes.end());
+	visitor v;
+	v.m_expect_value = false;
+	v.m_last_node_evttypes.clear();
+	filter->accept(&v);
+	out.insert(v.m_last_node_evttypes.begin(), v.m_last_node_evttypes.end());
 }
 
 void filter_evttype_resolver::evttypes(
-	shared_ptr<ast::expr> filter, set<uint16_t>& out)
+	shared_ptr<ast::expr> filter,
+	set<uint16_t>& out) const
 {
-	m_expect_value = false;
-	m_last_node_evttypes.clear();
-	filter.get()->accept(this);
-	out.insert(m_last_node_evttypes.begin(), m_last_node_evttypes.end());
+	visitor v;
+	v.m_expect_value = false;
+	v.m_last_node_evttypes.clear();
+	filter.get()->accept(&v);
+	out.insert(v.m_last_node_evttypes.begin(), v.m_last_node_evttypes.end());
 }
 
 // "and" nodes evttypes are the intersection of the evttypes of their children.
 // we initialize the set with "all event types"
-void filter_evttype_resolver::visit(ast::and_expr* e)
+void filter_evttype_resolver::visitor::visit(ast::and_expr* e)
 {
 	set<uint16_t> types, inters;
 	evttypes("", types);
@@ -96,7 +101,7 @@ void filter_evttype_resolver::visit(ast::and_expr* e)
 }
 
 // "or" nodes evttypes are the union of the evttypes their children
-void filter_evttype_resolver::visit(ast::or_expr* e)
+void filter_evttype_resolver::visitor::visit(ast::or_expr* e)
 {
 	set<uint16_t> types;
 	m_last_node_evttypes.clear();
@@ -108,14 +113,14 @@ void filter_evttype_resolver::visit(ast::or_expr* e)
 	m_last_node_evttypes = types;
 }
 
-void filter_evttype_resolver::visit(ast::not_expr* e)
+void filter_evttype_resolver::visitor::visit(ast::not_expr* e)
 {
 	m_last_node_evttypes.clear();
 	e->child->accept(this);
 	inversion(m_last_node_evttypes);
 }
 
-void filter_evttype_resolver::visit(ast::binary_check_expr* e)
+void filter_evttype_resolver::visitor::visit(ast::binary_check_expr* e)
 {
 	m_last_node_evttypes.clear();
 	if (e->field == "evt.type" && is_evttype_operator(e->op))
@@ -132,13 +137,13 @@ void filter_evttype_resolver::visit(ast::binary_check_expr* e)
 	evttypes("", m_last_node_evttypes);
 }
 
-void filter_evttype_resolver::visit(ast::unary_check_expr* e)
+void filter_evttype_resolver::visitor::visit(ast::unary_check_expr* e)
 {
 	m_last_node_evttypes.clear();
 	evttypes("", m_last_node_evttypes);
 }
 
-void filter_evttype_resolver::visit(ast::value_expr* e)
+void filter_evttype_resolver::visitor::visit(ast::value_expr* e)
 {
 	m_last_node_evttypes.clear();
 	if (m_expect_value)
@@ -149,7 +154,7 @@ void filter_evttype_resolver::visit(ast::value_expr* e)
 	evttypes("", m_last_node_evttypes);
 }
 
-void filter_evttype_resolver::visit(ast::list_expr* e)
+void filter_evttype_resolver::visitor::visit(ast::list_expr* e)
 {
 	m_last_node_evttypes.clear();
 	if (m_expect_value)

userspace/engine/filter_evttype_resolver.h

@@ -24,7 +24,7 @@ limitations under the License.
 /*!
 	\brief Helper class for finding event types
 */
-class filter_evttype_resolver: private libsinsp::filter::ast::expr_visitor
+class filter_evttype_resolver
 {
 public:
 	/*!
@@ -35,7 +35,10 @@ class filter_evttype_resolver: private libsinsp::filter::ast::expr_visitor
 		string is passed, all the available evttypes are collected
 		\param out The set to be filled with the evttypes
 	*/
-	void evttypes(std::string evtname, std::set<uint16_t>& out);
+	inline void evttypes(std::string evtname, std::set<uint16_t>& out) const
+	{
+		visitor().evttypes(evtname, out);
+	}
 
 	/*!
 		\brief Visits a filter AST and collects all the evttypes for which
@@ -45,25 +48,32 @@ class filter_evttype_resolver: private libsinsp::filter::ast::expr_visitor
 		\param filter The filter AST to be explored
 		\param out The set to be filled with the evttypes
 	*/
-	void evttypes(libsinsp::filter::ast::expr* filter, std::set<uint16_t>& out);
+	void evttypes(
+		libsinsp::filter::ast::expr* filter,
+		std::set<uint16_t>& out) const;
 
 	/*!
 		\brief Overloaded version of evttypes() that supports filters wrapped
 		in shared pointers
 	*/
-	void evttypes(std::shared_ptr<libsinsp::filter::ast::expr> filter,
-		std::set<uint16_t>& out);
+	void evttypes(
+		std::shared_ptr<libsinsp::filter::ast::expr> filter,
+		std::set<uint16_t>& out) const;
 
 private:
-	void visit(libsinsp::filter::ast::and_expr* e) override;
-	void visit(libsinsp::filter::ast::or_expr* e) override;
-	void visit(libsinsp::filter::ast::not_expr* e) override;
-	void visit(libsinsp::filter::ast::value_expr* e) override;
-	void visit(libsinsp::filter::ast::list_expr* e) override;
-	void visit(libsinsp::filter::ast::unary_check_expr* e) override;
-	void visit(libsinsp::filter::ast::binary_check_expr* e) override;
-	void inversion(std::set<uint16_t>& types);
+	struct visitor : public libsinsp::filter::ast::expr_visitor
+	{
+		bool m_expect_value;
+		std::set<uint16_t> m_last_node_evttypes;
 
-	bool m_expect_value;
-	std::set<uint16_t> m_last_node_evttypes;
+		void visit(libsinsp::filter::ast::and_expr* e) override;
+		void visit(libsinsp::filter::ast::or_expr* e) override;
+		void visit(libsinsp::filter::ast::not_expr* e) override;
+		void visit(libsinsp::filter::ast::value_expr* e) override;
+		void visit(libsinsp::filter::ast::list_expr* e) override;
+		void visit(libsinsp::filter::ast::unary_check_expr* e) override;
+		void visit(libsinsp::filter::ast::binary_check_expr* e) override;
+		void inversion(std::set<uint16_t>& types);
+		void evttypes(std::string evtname, std::set<uint16_t>& out);
+	};
 };

userspace/engine/filter_macro_resolver.cpp

@@ -21,29 +21,37 @@ using namespace libsinsp::filter;
 
 bool filter_macro_resolver::run(libsinsp::filter::ast::expr*& filter)
 {
+	visitor v;
 	m_unknown_macros.clear();
 	m_resolved_macros.clear();
-	m_last_node_changed = false;
-	m_last_node = filter;
-	filter->accept(this);
-	if (m_last_node_changed)
+	v.m_unknown_macros = &m_unknown_macros;
+	v.m_resolved_macros = &m_resolved_macros;
+	v.m_macros = &m_macros;
+	v.m_last_node_changed = false;
+	v.m_last_node = filter;
+	filter->accept(&v);
+	if (v.m_last_node_changed)
 	{
 		delete filter;
-		filter = m_last_node;
+		filter = v.m_last_node;
 	}
 	return !m_resolved_macros.empty();
 }
 
 bool filter_macro_resolver::run(std::shared_ptr<libsinsp::filter::ast::expr>& filter)
 {
+	visitor v;
 	m_unknown_macros.clear();
 	m_resolved_macros.clear();
-	m_last_node_changed = false;
-	m_last_node = filter.get();
-	filter->accept(this);
-	if (m_last_node_changed)
+	v.m_unknown_macros = &m_unknown_macros;
+	v.m_resolved_macros = &m_resolved_macros;
+	v.m_macros = &m_macros;
+	v.m_last_node_changed = false;
+	v.m_last_node = filter.get();
+	filter->accept(&v);
+	if (v.m_last_node_changed)
 	{
-		filter.reset(m_last_node);
+		filter.reset(v.m_last_node);
 	}
 	return !m_resolved_macros.empty();
 }
@@ -55,17 +63,17 @@ void filter_macro_resolver::set_macro(
 	m_macros[name] = macro;
 }
 
-set<string>& filter_macro_resolver::get_unknown_macros()
+const set<string>& filter_macro_resolver::get_unknown_macros() const
 {
 	return m_unknown_macros;
 }
 
-set<string>& filter_macro_resolver::get_resolved_macros()
+const set<string>& filter_macro_resolver::get_resolved_macros() const
 {
 	return m_resolved_macros;
 }
 
-void filter_macro_resolver::visit(ast::and_expr* e)
+void filter_macro_resolver::visitor::visit(ast::and_expr* e)
 {
 	for (size_t i = 0; i < e->children.size(); i++)
 	{
@@ -80,7 +88,7 @@ void filter_macro_resolver::visit(ast::and_expr* e)
 	m_last_node_changed = false;
 }
 
-void filter_macro_resolver::visit(ast::or_expr* e)
+void filter_macro_resolver::visitor::visit(ast::or_expr* e)
 {
 	for (size_t i = 0; i < e->children.size(); i++)
 	{
@@ -95,7 +103,7 @@ void filter_macro_resolver::visit(ast::or_expr* e)
 	m_last_node_changed = false;
 }
 
-void filter_macro_resolver::visit(ast::not_expr* e)
+void filter_macro_resolver::visitor::visit(ast::not_expr* e)
 {
 	e->child->accept(this);
 	if (m_last_node_changed)
@@ -107,43 +115,43 @@ void filter_macro_resolver::visit(ast::not_expr* e)
 	m_last_node_changed = false;
 }
 
-void filter_macro_resolver::visit(ast::list_expr* e)
+void filter_macro_resolver::visitor::visit(ast::list_expr* e)
 {
 	m_last_node = e;
 	m_last_node_changed = false;
 }
 
-void filter_macro_resolver::visit(ast::binary_check_expr* e)
+void filter_macro_resolver::visitor::visit(ast::binary_check_expr* e)
 {
 	// avoid exploring checks, so that we can be sure that each
 	// value_expr* node visited is a macro identifier
 	m_last_node = e;
 	m_last_node_changed = false;
 }
 
-void filter_macro_resolver::visit(ast::unary_check_expr* e)
+void filter_macro_resolver::visitor::visit(ast::unary_check_expr* e)
 {
 	m_last_node = e;
 	m_last_node_changed = false;
 }
 
-void filter_macro_resolver::visit(ast::value_expr* e)
+void filter_macro_resolver::visitor::visit(ast::value_expr* e)
 {
 	// we are supposed to get here only in case
 	// of identier-only children from either a 'not',
 	// an 'and' or an 'or'.
-	auto macro = m_macros.find(e->value);
-	if (macro != m_macros.end() && macro->second) // skip null-ptr macros
+	auto macro = m_macros->find(e->value);
+	if (macro != m_macros->end() && macro->second) // skip null-ptr macros
 	{
 		ast::expr* new_node = ast::clone(macro->second.get());
 		new_node->accept(this); // this sets m_last_node
 		m_last_node_changed = true;
-		m_resolved_macros.insert(e->value);
+		m_resolved_macros->insert(e->value);
 	}
 	else
 	{
 		m_last_node = e;
 		m_last_node_changed = false;
-		m_unknown_macros.insert(e->value);
+		m_unknown_macros->insert(e->value);
 	}
 }