update(engine): rename and improve sinsp filter macro resolvers

Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>

userspace/engine/filter_macro_resolver.cpp

@@ -34,7 +34,7 @@ bool filter_macro_resolver::run(libsinsp::filter::ast::expr*& filter)
 	return !m_resolved_macros.empty();
 }
 
-void filter_macro_resolver::define_macro(
+void filter_macro_resolver::set_macro(
 		string name,
 		shared_ptr<libsinsp::filter::ast::expr> macro)
 {
@@ -122,8 +122,8 @@ void filter_macro_resolver::visit(ast::value_expr* e)
 	auto macro = m_macros.find(e->value);
 	if (macro != m_macros.end())
 	{
-		// todo(jasondellaluce): should we visit down the new resolved AST too?
+		ast::expr* new_node = ast::clone(macro->second.get());
-		m_last_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);
 	}

userspace/engine/filter_macro_resolver.h

@@ -31,7 +31,7 @@ class filter_macro_resolver: private libsinsp::filter::ast::expr_visitor
 	public:
 		/*!
 			\brief Visits a filter AST and substitutes macro references
-			according with all the definitions added through define_macro(),
+			according with all the definitions added through set_macro(),
 			by replacing the reference with a clone of the macro AST.
 			\param filter The filter AST to be processed. Note that the pointer
 			is passed by reference and be modified in order to apply
@@ -48,7 +48,7 @@ class filter_macro_resolver: private libsinsp::filter::ast::expr_visitor
 			\param name The name of the macro.
 			\param macro The AST of the macro.
 		*/
-		void define_macro(
+		void set_macro(
 			std::string name,
 			std::shared_ptr<libsinsp::filter::ast::expr> macro);
 
@@ -63,7 +63,7 @@ class filter_macro_resolver: private libsinsp::filter::ast::expr_visitor
 			\brief Returns a set containing the names of all the macros
 			that remained unresolved during the last invocation of run().
 			A macro remains unresolved if it is found inside the processed
-			filter but it was not defined with define_macro();
+			filter but it was not defined with set_macro();
 		*/
 		std::set<std::string>& get_unknown_macros();
 		

userspace/engine/lua_filter_helper.cpp

@@ -17,7 +17,6 @@ limitations under the License.
 #include <sinsp.h>
 #include "lua_filter_helper.h"
 #include "filter_macro_resolver.h"
-#include "filter_list_resolver.h"
 #include "rules.h"
 
 using namespace std;
@@ -35,7 +34,6 @@ const static struct luaL_Reg ll_filter_helper[] =
 {
 	{"compile_filter", &lua_filter_helper::compile_filter},
 	{"parse_filter", &lua_filter_helper::parse_filter},
-	{"expand_list", &lua_filter_helper::expand_list},
 	{"expand_macro", &lua_filter_helper::expand_macro},
 	{"find_unknown_macro", &lua_filter_helper::find_unknown_macro},
 	{"clone_ast", &lua_filter_helper::clone_ast},
@@ -112,37 +110,6 @@ int lua_filter_helper::compile_filter(lua_State *ls)
 	return 2;
 }
 
-int lua_filter_helper::expand_list(lua_State *ls)
-{
-	if (! lua_islightuserdata(ls, -3) ||	// ast
-		! lua_isstring(ls, -2) ||		   // name
-		! lua_istable(ls, -1))			  // values
-	{
-		lua_pushstring(ls, "invalid arguments passed to expand_list()");
-		lua_error(ls);
-	}
-
-	ast::expr* ast = (ast::expr*) lua_topointer(ls, -3);
-	std::string name = lua_tostring(ls, -2);
-	vector<string> values;
-	// first key
-	lua_pushnil(ls);
-	while (lua_next(ls, -2) != 0) {
-		// key is at index -2, value is at index
-		// -1. We want the values.
-		values.push_back(lua_tostring(ls, -1));
-		// Remove value, keep key for next iteration
-		lua_pop(ls, 1);
-	}
-	
-	filter_list_resolver resolver;
-	resolver.define_list(name, values);
-	resolver.process(ast);
-	lua_pushboolean(ls, !resolver.get_resolved_lists().empty());
-	lua_pushlightuserdata(ls, ast);
-	return 2;
-}
-
 int lua_filter_helper::expand_macro(lua_State *ls)
 {
 	if (! lua_islightuserdata(ls, -3) ||	// ast
@@ -157,10 +124,14 @@ int lua_filter_helper::expand_macro(lua_State *ls)
 	std::string name = lua_tostring(ls, -2);
 	ast::expr* macro = (ast::expr*) lua_topointer(ls, -1);
 
+	// For now we need to clone the macro AST because the current Lua
+	// rule-loader implementation manages the pointer lifecycle manually,
+	// and it's not compatible with shared_ptr.
+	shared_ptr<ast::expr> macro_clone(ast::clone(macro));
 	filter_macro_resolver resolver;
-	resolver.define_macro(name, macro);
+	resolver.set_macro(name, macro_clone);
-	resolver.process(ast);
+	bool resolved = resolver.run(ast);
-	lua_pushboolean(ls, !resolver.get_resolved_macros().empty());
+	lua_pushboolean(ls, resolved);
 	lua_pushlightuserdata(ls, ast);
 	return 2;
 }
@@ -175,8 +146,10 @@ int lua_filter_helper::find_unknown_macro(lua_State *ls)
 
 	ast::expr* ast = (ast::expr*) lua_topointer(ls, -1);
 
+	// Running a macro resolver without defining any macro allows
+	// us to spot all the still-unresolved macros in an AST.
 	filter_macro_resolver resolver;
-	resolver.process(ast);
+	resolver.run(ast);
 	if (!resolver.get_unknown_macros().empty())
 	{
 		lua_pushboolean(ls, true);

userspace/engine/lua_filter_helper.h

@@ -29,7 +29,6 @@ public:
 	static void init(lua_State *ls);
 	static int compile_filter(lua_State *ls);
 	static int parse_filter(lua_State *ls);
-	static int expand_list(lua_State *ls);
 	static int expand_macro(lua_State *ls);
 	static int find_unknown_macro(lua_State *ls);
 	static int clone_ast(lua_State *ls);