Skip to content

Commit

Permalink
cleanup: fix several warnings from a Clang build
Browse files Browse the repository at this point in the history
Signed-off-by: Federico Aponte <[email protected]>
  • Loading branch information
federico-sysdig committed Dec 5, 2023
1 parent 0ba0dd8 commit 9edeb6d
Show file tree
Hide file tree
Showing 19 changed files with 93 additions and 172 deletions.
44 changes: 22 additions & 22 deletions unit_tests/engine/test_filter_macro_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,17 @@ TEST(MacroResolver, should_resolve_macros_on_a_filter_AST)
{
libsinsp::filter::ast::pos_info macro_pos(12, 85, 27);

std::shared_ptr<libsinsp::filter::ast::expr> macro = std::move(libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists"));
std::shared_ptr<libsinsp::filter::ast::expr> macro = libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists");

std::vector<std::unique_ptr<libsinsp::filter::ast::expr>> filter_and;
filter_and.push_back(libsinsp::filter::ast::unary_check_expr::create("evt.name", "", "exists"));
filter_and.push_back(libsinsp::filter::ast::not_expr::create(libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos)));
std::shared_ptr<libsinsp::filter::ast::expr> filter = std::move(libsinsp::filter::ast::and_expr::create(filter_and));
std::shared_ptr<libsinsp::filter::ast::expr> filter = libsinsp::filter::ast::and_expr::create(filter_and);

std::vector<std::unique_ptr<libsinsp::filter::ast::expr>> expected_and;
expected_and.push_back(libsinsp::filter::ast::unary_check_expr::create("evt.name", "", "exists"));
expected_and.push_back(libsinsp::filter::ast::not_expr::create(clone(macro.get())));
std::shared_ptr<libsinsp::filter::ast::expr> expected = std::move(libsinsp::filter::ast::and_expr::create(expected_and));
std::shared_ptr<libsinsp::filter::ast::expr> expected = libsinsp::filter::ast::and_expr::create(expected_and);

filter_macro_resolver resolver;
resolver.set_macro(MACRO_NAME, macro);
Expand All @@ -71,9 +71,9 @@ TEST(MacroResolver, should_resolve_macros_on_a_filter_AST_single_node)
{
libsinsp::filter::ast::pos_info macro_pos(12, 85, 27);

std::shared_ptr<libsinsp::filter::ast::expr> macro = std::move(libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists"));
std::shared_ptr<libsinsp::filter::ast::expr> macro = libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists");

std::shared_ptr<libsinsp::filter::ast::expr> filter = std::move(libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos));
std::shared_ptr<libsinsp::filter::ast::expr> filter = libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos);

filter_macro_resolver resolver;
resolver.set_macro(MACRO_NAME, macro);
Expand Down Expand Up @@ -102,18 +102,18 @@ TEST(MacroResolver, should_resolve_macros_on_a_filter_AST_multiple_macros)
libsinsp::filter::ast::pos_info a_macro_pos(11, 75, 43);
libsinsp::filter::ast::pos_info b_macro_pos(91, 21, 9);

std::shared_ptr<libsinsp::filter::ast::expr> a_macro = std::move(libsinsp::filter::ast::unary_check_expr::create("one.field", "", "exists"));
std::shared_ptr<libsinsp::filter::ast::expr> b_macro = std::move(libsinsp::filter::ast::unary_check_expr::create("another.field", "", "exists"));
std::shared_ptr<libsinsp::filter::ast::expr> a_macro = libsinsp::filter::ast::unary_check_expr::create("one.field", "", "exists");
std::shared_ptr<libsinsp::filter::ast::expr> b_macro = libsinsp::filter::ast::unary_check_expr::create("another.field", "", "exists");

std::vector<std::unique_ptr<libsinsp::filter::ast::expr>> filter_or;
filter_or.push_back(libsinsp::filter::ast::value_expr::create(MACRO_A_NAME, a_macro_pos));
filter_or.push_back(libsinsp::filter::ast::value_expr::create(MACRO_B_NAME, b_macro_pos));
std::shared_ptr<libsinsp::filter::ast::expr> filter = std::move(libsinsp::filter::ast::or_expr::create(filter_or));
std::shared_ptr<libsinsp::filter::ast::expr> filter = libsinsp::filter::ast::or_expr::create(filter_or);

std::vector<std::unique_ptr<libsinsp::filter::ast::expr>> expected_or;
expected_or.push_back(clone(a_macro.get()));
expected_or.push_back(clone(b_macro.get()));
std::shared_ptr<libsinsp::filter::ast::expr> expected_filter = std::move(libsinsp::filter::ast::or_expr::create(expected_or));
std::shared_ptr<libsinsp::filter::ast::expr> expected_filter = libsinsp::filter::ast::or_expr::create(expected_or);

filter_macro_resolver resolver;
resolver.set_macro(MACRO_A_NAME, a_macro);
Expand Down Expand Up @@ -149,17 +149,17 @@ TEST(MacroResolver, should_resolve_macros_on_a_filter_AST_nested_macros)
std::vector<std::unique_ptr<libsinsp::filter::ast::expr>> a_macro_and;
a_macro_and.push_back(libsinsp::filter::ast::unary_check_expr::create("one.field", "", "exists"));
a_macro_and.push_back(libsinsp::filter::ast::value_expr::create(MACRO_B_NAME, b_macro_pos));
std::shared_ptr<libsinsp::filter::ast::expr> a_macro = std::move(libsinsp::filter::ast::and_expr::create(a_macro_and));
std::shared_ptr<libsinsp::filter::ast::expr> a_macro = libsinsp::filter::ast::and_expr::create(a_macro_and);

std::shared_ptr<libsinsp::filter::ast::expr> b_macro = std::move(
libsinsp::filter::ast::unary_check_expr::create("another.field", "", "exists"));
std::shared_ptr<libsinsp::filter::ast::expr> b_macro =
libsinsp::filter::ast::unary_check_expr::create("another.field", "", "exists");

std::shared_ptr<libsinsp::filter::ast::expr> filter = std::move(libsinsp::filter::ast::value_expr::create(MACRO_A_NAME, a_macro_pos));
std::shared_ptr<libsinsp::filter::ast::expr> filter = libsinsp::filter::ast::value_expr::create(MACRO_A_NAME, a_macro_pos);

std::vector<std::unique_ptr<libsinsp::filter::ast::expr>> expected_and;
expected_and.push_back(libsinsp::filter::ast::unary_check_expr::create("one.field", "", "exists"));
expected_and.push_back(libsinsp::filter::ast::unary_check_expr::create("another.field", "", "exists"));
std::shared_ptr<libsinsp::filter::ast::expr> expected_filter = std::move(libsinsp::filter::ast::and_expr::create(expected_and));
std::shared_ptr<libsinsp::filter::ast::expr> expected_filter = libsinsp::filter::ast::and_expr::create(expected_and);

filter_macro_resolver resolver;
resolver.set_macro(MACRO_A_NAME, a_macro);
Expand Down Expand Up @@ -196,7 +196,7 @@ TEST(MacroResolver, should_find_unknown_macros)
std::vector<std::unique_ptr<libsinsp::filter::ast::expr>> filter_and;
filter_and.push_back(libsinsp::filter::ast::unary_check_expr::create("evt.name", "", "exists"));
filter_and.push_back(libsinsp::filter::ast::not_expr::create(libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos)));
std::shared_ptr<libsinsp::filter::ast::expr> filter = std::move(libsinsp::filter::ast::and_expr::create(filter_and));
std::shared_ptr<libsinsp::filter::ast::expr> filter = libsinsp::filter::ast::and_expr::create(filter_and);

filter_macro_resolver resolver;
ASSERT_FALSE(resolver.run(filter));
Expand All @@ -214,9 +214,9 @@ TEST(MacroResolver, should_find_unknown_nested_macros)
std::vector<std::unique_ptr<libsinsp::filter::ast::expr>> a_macro_and;
a_macro_and.push_back(libsinsp::filter::ast::unary_check_expr::create("one.field", "", "exists"));
a_macro_and.push_back(libsinsp::filter::ast::value_expr::create(MACRO_B_NAME, b_macro_pos));
std::shared_ptr<libsinsp::filter::ast::expr> a_macro = std::move(libsinsp::filter::ast::and_expr::create(a_macro_and));
std::shared_ptr<libsinsp::filter::ast::expr> a_macro = libsinsp::filter::ast::and_expr::create(a_macro_and);

std::shared_ptr<libsinsp::filter::ast::expr> filter = std::move(libsinsp::filter::ast::value_expr::create(MACRO_A_NAME, a_macro_pos));
std::shared_ptr<libsinsp::filter::ast::expr> filter = libsinsp::filter::ast::value_expr::create(MACRO_A_NAME, a_macro_pos);
auto expected_filter = clone(a_macro.get());

filter_macro_resolver resolver;
Expand All @@ -237,9 +237,9 @@ TEST(MacroResolver, should_undefine_macro)
libsinsp::filter::ast::pos_info macro_pos_1(12, 9, 3);
libsinsp::filter::ast::pos_info macro_pos_2(9, 6, 3);

std::shared_ptr<libsinsp::filter::ast::expr> macro = std::move(libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists"));
std::shared_ptr<libsinsp::filter::ast::expr> a_filter = std::move(libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos_1));
std::shared_ptr<libsinsp::filter::ast::expr> b_filter = std::move(libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos_2));
std::shared_ptr<libsinsp::filter::ast::expr> macro = libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists");
std::shared_ptr<libsinsp::filter::ast::expr> a_filter = libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos_1);
std::shared_ptr<libsinsp::filter::ast::expr> b_filter = libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos_2);
filter_macro_resolver resolver;

resolver.set_macro(MACRO_NAME, macro);
Expand All @@ -262,8 +262,8 @@ TEST(MacroResolver, should_undefine_macro)
TEST(MacroResolver, should_clone_macro_AST)
{
libsinsp::filter::ast::pos_info macro_pos(5, 2, 8888);
std::shared_ptr<libsinsp::filter::ast::unary_check_expr> macro = std::move(libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists"));
std::shared_ptr<libsinsp::filter::ast::expr> filter = std::move(libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos));
std::shared_ptr<libsinsp::filter::ast::unary_check_expr> macro = libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists");
std::shared_ptr<libsinsp::filter::ast::expr> filter = libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos);
filter_macro_resolver resolver;

resolver.set_macro(MACRO_NAME, macro);
Expand Down
54 changes: 27 additions & 27 deletions userspace/engine/falco_engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ std::unique_ptr<load_result> falco_engine::load_rules(const std::string &rules_c
// read rules YAML file and collect its definitions
rule_loader::reader reader;
if (reader.read(cfg, m_rule_collector))
{
{
// compile the definitions (resolve macro/list refs, exceptions, ...)
m_last_compile_output = std::make_unique<rule_loader::compiler::compile_output>();
rule_loader::compiler().compile(cfg, m_rule_collector, *m_last_compile_output.get());
Expand Down Expand Up @@ -384,7 +384,7 @@ libsinsp::events::set<ppm_sc_code> falco_engine::sc_codes_for_ruleset(const std:
{
return find_source(source)->ruleset->enabled_sc_codes(find_ruleset_id(ruleset));
}

libsinsp::events::set<ppm_event_code> falco_engine::event_codes_for_ruleset(const std::string &source, const std::string &ruleset)
{
return find_source(source)->ruleset->enabled_event_codes(find_ruleset_id(ruleset));
Expand Down Expand Up @@ -550,7 +550,7 @@ nlohmann::json falco_engine::describe_rule(std::string *rule, const std::vector<
alternatives.push_back(std::move(alternative));
}
r["alternatives"] = std::move(alternatives);

plugin_versions.push_back(std::move(r));
}
output["required_plugin_versions"] = std::move(plugin_versions);
Expand All @@ -565,7 +565,7 @@ nlohmann::json falco_engine::describe_rule(std::string *rule, const std::vector<
rules_array.push_back(std::move(rule));
}
output["rules"] = std::move(rules_array);

// Store information about macros
nlohmann::json macros_array = nlohmann::json::array();
for(const auto &m : m_last_compile_output->macros)
Expand All @@ -577,14 +577,14 @@ nlohmann::json falco_engine::describe_rule(std::string *rule, const std::vector<
}
output["macros"] = std::move(macros_array);

// Store information about lists
// Store information about lists
nlohmann::json lists_array = nlohmann::json::array();
for(const auto &l : m_last_compile_output->lists)
{
auto info = m_rule_collector.lists().at(l.name);
nlohmann::json list;
get_json_details(list, l, *info, plugins);
lists_array.push_back(std::move(list));
lists_array.push_back(std::move(list));
}
output["lists"] = std::move(lists_array);
}
Expand Down Expand Up @@ -619,12 +619,12 @@ void falco_engine::get_json_details(
// Fill general rule information
rule_info["name"] = r.name;
rule_info["condition"] = info.cond;
rule_info["priority"] = std::move(format_priority(r.priority, false));
rule_info["priority"] = format_priority(r.priority, false);
rule_info["output"] = info.output;
rule_info["description"] = r.description;
rule_info["enabled"] = info.enabled;
rule_info["source"] = r.source;
rule_info["tags"] = std::move(sequence_to_json_array(info.tags));
rule_info["tags"] = sequence_to_json_array(info.tags);
out["info"] = std::move(rule_info);

// Parse rule condition and build the non-compiled AST
Expand All @@ -648,19 +648,19 @@ void falco_engine::get_json_details(
filter_details_resolver().run(ast.get(), details);
filter_details_resolver().run(r.condition.get(), compiled_details);

out["details"]["macros"] = std::move(sequence_to_json_array(details.macros));
out["details"]["lists"] = std::move(sequence_to_json_array(details.lists));
out["details"]["condition_operators"] = std::move(sequence_to_json_array(compiled_details.operators));
out["details"]["condition_fields"] = std::move(sequence_to_json_array(compiled_details.fields));
out["details"]["macros"] = sequence_to_json_array(details.macros);
out["details"]["lists"] = sequence_to_json_array(details.lists);
out["details"]["condition_operators"] = sequence_to_json_array(compiled_details.operators);
out["details"]["condition_fields"] = sequence_to_json_array(compiled_details.fields);

// Get fields from output string
auto fmt = create_formatter(r.source, r.output);
std::vector<std::string> out_fields;
fmt->get_field_names(out_fields);
out["details"]["output_fields"] = std::move(sequence_to_json_array(out_fields));
out["details"]["output_fields"] = sequence_to_json_array(out_fields);

// Get fields from exceptions
out["details"]["exception_fields"] = std::move(sequence_to_json_array(r.exception_fields));
out["details"]["exception_fields"] = sequence_to_json_array(r.exception_fields);

// Get names and operators from exceptions
std::unordered_set<std::string> exception_names;
Expand Down Expand Up @@ -689,18 +689,18 @@ void falco_engine::get_json_details(
else
{
exception_operators.insert(e.comps.item);
}
}
}
out["details"]["exception_names"] = std::move(sequence_to_json_array(exception_names));
out["details"]["exception_operators"] = std::move(sequence_to_json_array(exception_operators));
out["details"]["exception_names"] = sequence_to_json_array(exception_names);
out["details"]["exception_operators"] = sequence_to_json_array(exception_operators);

// Store event types
nlohmann::json events;
get_json_evt_types(events, info.source, r.condition.get());
out["details"]["events"] = std::move(events);

// Store compiled condition and output
out["details"]["condition_compiled"] = std::move(libsinsp::filter::ast::as_string(r.condition.get()));
out["details"]["condition_compiled"] = libsinsp::filter::ast::as_string(r.condition.get());
out["details"]["output_compiled"] = r.output;

// Compute the plugins that are actually used by this rule. This is involves:
Expand Down Expand Up @@ -750,26 +750,26 @@ void falco_engine::get_json_details(
filter_details_resolver().run(m.condition.get(), compiled_details);

out["details"]["used"] = m.used;
out["details"]["macros"] = std::move(sequence_to_json_array(details.macros));
out["details"]["lists"] = std::move(sequence_to_json_array(details.lists));
out["details"]["condition_operators"] = std::move(sequence_to_json_array(compiled_details.operators));
out["details"]["condition_fields"] = std::move(sequence_to_json_array(compiled_details.fields));
out["details"]["macros"] = sequence_to_json_array(details.macros);
out["details"]["lists"] = sequence_to_json_array(details.lists);
out["details"]["condition_operators"] = sequence_to_json_array(compiled_details.operators);
out["details"]["condition_fields"] = sequence_to_json_array(compiled_details.fields);

// Store event types
nlohmann::json events;
get_json_evt_types(events, "", m.condition.get());
out["details"]["events"] = std::move(events);

// Store compiled condition
out["details"]["condition_compiled"] = std::move(libsinsp::filter::ast::as_string(m.condition.get()));
out["details"]["condition_compiled"] = libsinsp::filter::ast::as_string(m.condition.get());

// Compute the plugins that are actually used by this macro.
// Note: macros have no specific source, we need to set an empty list of used
// plugins because we can't be certain about their actual usage. For example,
// if a macro uses a plugin's field, we can't be sure which plugin actually
// is used until we resolve the macro ref in a rule providing a source for
// disambiguation.
out["details"]["plugins"] = std::move(nlohmann::json::array());
out["details"]["plugins"] = nlohmann::json::array();
}

void falco_engine::get_json_details(
Expand Down Expand Up @@ -800,9 +800,9 @@ void falco_engine::get_json_details(
list_info["items"] = std::move(items);
out["info"] = std::move(list_info);
out["details"]["used"] = l.used;
out["details"]["lists"] = std::move(sequence_to_json_array(lists));
out["details"]["items_compiled"] = std::move(sequence_to_json_array(l.items));
out["details"]["plugins"] = std::move(nlohmann::json::array()); // always empty
out["details"]["lists"] = sequence_to_json_array(lists);
out["details"]["items_compiled"] = sequence_to_json_array(l.items);
out["details"]["plugins"] = nlohmann::json::array(); // always empty
}

void falco_engine::get_json_evt_types(
Expand Down
14 changes: 6 additions & 8 deletions userspace/engine/filter_details_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ limitations under the License.
#include <unordered_set>
#include <unordered_map>

struct filter_details
struct filter_details
{
// input macros and lists
std::unordered_set<std::string> known_macros;
std::unordered_set<std::string> known_lists;

// output details
std::unordered_set<std::string> fields;
std::unordered_set<std::string> macros;
Expand All @@ -47,25 +47,23 @@ class filter_details_resolver
/*!
\brief Visits a filter AST and stores details about macros, lists,
fields and operators used.
\param filter The filter AST to be processed.
\param details Helper structure used to state known macros and
\param filter The filter AST to be processed.
\param details Helper structure used to state known macros and
lists on input, and to store all the retrieved details as output.
*/
void run(libsinsp::filter::ast::expr* filter,
filter_details& details);

private:
struct visitor : public libsinsp::filter::ast::expr_visitor
{
visitor(filter_details& details) :
visitor(filter_details& details) :
m_details(details),
m_expect_list(false),
m_expect_macro(false),
m_expect_evtname(false) {}
visitor(visitor&&) = default;
visitor& operator = (visitor&&) = default;
visitor(const visitor&) = delete;
visitor& operator = (const visitor&) = delete;

void visit(libsinsp::filter::ast::and_expr* e) override;
void visit(libsinsp::filter::ast::or_expr* e) override;
Expand Down
6 changes: 1 addition & 5 deletions userspace/engine/filter_macro_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class filter_macro_resolver

/*!
\brief used in get_{resolved,unknown}_macros and get_errors
to represent an identifier/string value along with an AST position.
to represent an identifier/string value along with an AST position.
*/
typedef std::pair<std::string,libsinsp::filter::ast::pos_info> value_info;

Expand Down Expand Up @@ -103,10 +103,6 @@ class filter_macro_resolver
m_unknown_macros(unknown_macros),
m_resolved_macros(resolved_macros),
m_macros(macros) {}
visitor(visitor&&) = default;
visitor& operator = (visitor&&) = default;
visitor(const visitor&) = delete;
visitor& operator = (const visitor&) = delete;

std::vector<std::string> m_macros_path;
std::unique_ptr<libsinsp::filter::ast::expr> m_node_substitute;
Expand Down
8 changes: 3 additions & 5 deletions userspace/engine/rule_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,10 +562,8 @@ rule_loader::rule_load_exception::~rule_load_exception()
{
}

const char* rule_loader::rule_load_exception::what()
const char* rule_loader::rule_load_exception::what() const noexcept
{
errstr = falco::load_result::error_code_str(ec) + ": "
+ msg.c_str();

return errstr.c_str();
// const + noexcept: can't use functions that change the object or throw
return msg.c_str();
}
Loading

0 comments on commit 9edeb6d

Please sign in to comment.