refactor: test AtomicSignalHandler.handle_once_wait_consistency

Signed-off-by: Federico Aponte <federico.aponte@sysdig.com>

unit_tests/engine/test_add_source.cpp

@@ -54,12 +54,9 @@ TEST(AddSource, basic)
 	sinsp inspector;
 	sinsp_filter_check_list filterchecks;
 
-	auto filter_factory = std::shared_ptr<sinsp_filter_factory>(
+	auto filter_factory = std::make_shared<sinsp_filter_factory>(&inspector, filterchecks);
-		new sinsp_filter_factory(&inspector, filterchecks));
+	auto formatter_factory = std::make_shared<sinsp_evt_formatter_factory>(&inspector, filterchecks);
-	auto formatter_factory = std::shared_ptr<sinsp_evt_formatter_factory>(
+	auto ruleset_factory = std::make_shared<test_ruleset_factory>(filter_factory);
-		new sinsp_evt_formatter_factory(&inspector, filterchecks));
-	test_ruleset_factory *test_factory = new test_ruleset_factory(filter_factory);
-	auto ruleset_factory = std::shared_ptr<filter_ruleset_factory>(test_factory);
 
 	falco_source syscall_source;
 	syscall_source.name = syscall_source_name;
@@ -84,6 +81,6 @@ TEST(AddSource, basic)
 	ASSERT_EQ(engine.ruleset_factory_for_source(syscall_source_name), ruleset_factory);
 	ASSERT_EQ(engine.ruleset_factory_for_source(source_idx), ruleset_factory);
 
-	ASSERT_EQ(engine.ruleset_for_source(syscall_source_name), test_factory->ruleset);
+	ASSERT_EQ(engine.ruleset_for_source(syscall_source_name), ruleset_factory->ruleset);
-	ASSERT_EQ(engine.ruleset_for_source(source_idx), test_factory->ruleset);
+	ASSERT_EQ(engine.ruleset_for_source(source_idx), ruleset_factory->ruleset);
 }

unit_tests/engine/test_alt_rule_loader.cpp

@@ -163,8 +163,7 @@ public:
 
 	inline std::shared_ptr<filter_ruleset> new_ruleset() override
 	{
-		std::shared_ptr<filter_ruleset> ret(new test_ruleset(m_filter_factory));
+		return std::make_shared<test_ruleset>(m_filter_factory);
-		return ret;
 	}
 
 	std::shared_ptr<sinsp_filter_factory> m_filter_factory;
@@ -235,12 +234,9 @@ static std::shared_ptr<rule_loader::configuration> create_configuration(sinsp& i
 									sinsp_filter_check_list& filterchecks,
 									indexed_vector<falco_source>& sources)
 {
-	auto filter_factory = std::shared_ptr<sinsp_filter_factory>(
+	auto filter_factory = std::make_shared<sinsp_filter_factory>(&inspector, filterchecks);
-		new sinsp_filter_factory(&inspector, filterchecks));
+	auto formatter_factory = std::make_shared<sinsp_evt_formatter_factory>(&inspector, filterchecks);
-	auto formatter_factory = std::shared_ptr<sinsp_evt_formatter_factory>(
+	auto ruleset_factory = std::make_shared<evttype_index_ruleset_factory>(filter_factory);
-		new sinsp_evt_formatter_factory(&inspector, filterchecks));
-	auto ruleset_factory = std::shared_ptr<filter_ruleset_factory>(
-		new evttype_index_ruleset_factory(filter_factory));
 
 	falco_source syscall_source;
 	syscall_source.name = syscall_source_name;
@@ -251,12 +247,9 @@ static std::shared_ptr<rule_loader::configuration> create_configuration(sinsp& i
 
 	sources.insert(syscall_source, syscall_source_name);
 
-	std::shared_ptr<rule_loader::configuration> configuration =
+	return std::make_shared<rule_loader::configuration>(content,
-		std::make_shared<rule_loader::configuration>(content,
+	                                                    sources,
-							     sources,
+	                                                    "test configuration");
-							     "test configuration");
-
-	return configuration;
 }
 
 static void load_rules(sinsp& inspector,
@@ -330,22 +323,15 @@ TEST(engine_loader_alt_loader, falco_engine_alternate_loader)
 	sinsp inspector;
 	sinsp_filter_check_list filterchecks;
 
-	auto filter_factory = std::shared_ptr<sinsp_filter_factory>(
+	auto filter_factory = std::make_shared<sinsp_filter_factory>(&inspector, filterchecks);
-		new sinsp_filter_factory(&inspector, filterchecks));
+	auto formatter_factory = std::make_shared<sinsp_evt_formatter_factory>(&inspector, filterchecks);
-	auto formatter_factory = std::shared_ptr<sinsp_evt_formatter_factory>(
+	auto ruleset_factory = std::make_shared<test_ruleset_factory>(filter_factory);
-		new sinsp_evt_formatter_factory(&inspector, filterchecks));
-	auto ruleset_factory = std::shared_ptr<filter_ruleset_factory>(
-		new test_ruleset_factory(filter_factory));
 
-	engine.add_source(syscall_source_name,
+	engine.add_source(syscall_source_name, filter_factory, formatter_factory, ruleset_factory);
-			  filter_factory,
-			  formatter_factory,
-			  ruleset_factory);
 
-	std::shared_ptr<rule_loader::reader> reader(new test_reader());
+	auto reader = std::make_shared<test_reader>();
-	test_collector* test_col = new test_collector();
+	auto collector = std::make_shared<test_collector>();
-	std::shared_ptr<rule_loader::collector> collector(test_col);
+	auto compiler = std::make_shared<test_compiler>();
-	std::shared_ptr<rule_loader::compiler> compiler(new test_compiler());
 
 	engine.set_rule_reader(reader);
 	engine.set_rule_collector(collector);
@@ -357,7 +343,7 @@ TEST(engine_loader_alt_loader, falco_engine_alternate_loader)
 
 	engine.load_rules(content, "test_rules.yaml");
 
-	EXPECT_EQ(test_col->test_object_infos.size(), 2);
+	EXPECT_EQ(collector->test_object_infos.size(), 2);
 
 	std::shared_ptr<filter_ruleset> ruleset = engine.ruleset_for_source(syscall_source_name);
 	std::set<std::string>& defined_properties = std::dynamic_pointer_cast<test_ruleset>(ruleset)->defined_properties;

unit_tests/engine/test_plugin_requirements.cpp

@@ -23,15 +23,15 @@ static bool check_requirements(std::string& err,
 			       const std::vector<falco_engine::plugin_version_requirement>& plugins,
 			       const std::string& ruleset_content)
 {
-	std::unique_ptr<falco_engine> e(new falco_engine());
+	falco_engine e;
 	falco::load_result::rules_contents_t c = {{"test", ruleset_content}};
 
-	auto res = e->load_rules(c.begin()->second, c.begin()->first);
+	auto res = e.load_rules(c.begin()->second, c.begin()->first);
 	if(!res->successful())
 	{
 		return false;
 	}
-	return e->check_plugin_requirements(plugins, err);
+	return e.check_plugin_requirements(plugins, err);
 }
 
 TEST(PluginRequirements, check_plugin_requirements_success)

unit_tests/engine/test_rulesets.cpp

@@ -25,30 +25,26 @@ limitations under the License.
 /* Helpers methods */
 static std::shared_ptr<sinsp_filter_factory> create_factory(sinsp* inspector, filter_check_list& list)
 {
-	std::shared_ptr<sinsp_filter_factory> ret(new sinsp_filter_factory(inspector, list));
+	return std::make_shared<sinsp_filter_factory>(inspector, list);
-	return ret;
 }
 
 static std::shared_ptr<filter_ruleset> create_ruleset(std::shared_ptr<sinsp_filter_factory> f)
 {
-	std::shared_ptr<filter_ruleset> ret(new evttype_index_ruleset(f));
+	return std::make_shared<evttype_index_ruleset>(f);
-	return ret;
 }
 
 static std::shared_ptr<libsinsp::filter::ast::expr> create_ast(std::shared_ptr<sinsp_filter_factory> f)
 {
 	libsinsp::filter::parser parser("evt.type=open");
-	std::shared_ptr<libsinsp::filter::ast::expr> ret(parser.parse());
+	return parser.parse();
-	return ret;
 }
 
 static std::shared_ptr<sinsp_filter> create_filter(
 	std::shared_ptr<sinsp_filter_factory> f,
-	std::shared_ptr<libsinsp::filter::ast::expr> ast)
+	libsinsp::filter::ast::expr* ast)
 {
-	sinsp_filter_compiler compiler(f, ast.get());
+	sinsp_filter_compiler compiler(f, ast);
-	std::shared_ptr<sinsp_filter> filter(compiler.compile());
+	return std::shared_ptr<sinsp_filter>(compiler.compile());
-	return filter;
 }
 
 TEST(Ruleset, enable_disable_rules_using_names)
@@ -59,7 +55,7 @@ TEST(Ruleset, enable_disable_rules_using_names)
 	auto f = create_factory(&inspector, filterlist);
 	auto r = create_ruleset(f);
 	auto ast = create_ast(f);
-	auto filter = create_filter(f, ast);
+	auto filter = create_filter(f, ast.get());
 
 	falco_rule rule_A = {};
 	rule_A.name = "rule_A";
@@ -128,7 +124,7 @@ TEST(Ruleset, enable_disable_rules_using_tags)
 	auto f = create_factory(&inspector, filterlist);
 	auto r = create_ruleset(f);
 	auto ast = create_ast(f);
-	auto filter = create_filter(f, ast);
+	auto filter = create_filter(f, ast.get());
 
 	falco_rule rule_A = {};
 	rule_A.name = "rule_A";

unit_tests/falco/test_atomic_signal_handler.cpp

@@ -15,16 +15,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-#include <gtest/gtest.h>
-#include <future>
-#include <thread>
-#include <vector>
-#include <memory>
-#include <chrono>
- 
 #include <falco/atomic_signal_handler.h>
 #include <falco/logger.h>
 
+#include <gtest/gtest.h>
+
+#include <chrono>
+#include <future>
+#include <memory>
+#include <vector>
+
 TEST(AtomicSignalHandler, lock_free_implementation)
 {
 	ASSERT_TRUE(falco::atomic_signal_handler().is_lock_free());
@@ -33,55 +33,50 @@ TEST(AtomicSignalHandler, lock_free_implementation)
 TEST(AtomicSignalHandler, handle_once_wait_consistency)
 {
 	constexpr const auto thread_num = 10;
-	constexpr const auto thread_wait_sec = 2;
+	constexpr const std::chrono::seconds thread_wait_sec{2};
-	constexpr const auto handler_wait_sec = 1;
+	constexpr const std::chrono::seconds handler_wait_sec{1};
 
 	// have a shared signal handler
 	falco::atomic_signal_handler handler;
 
 	// launch a bunch of threads all syncing on the same handler
-	typedef struct
+	struct task_result_t
 	{
 		bool handled;
-		uint64_t duration_secs;
+		std::chrono::seconds duration_secs;
-	} task_result_t;
+	};
+
 	std::vector<std::future<task_result_t>> futures;
-	std::vector<std::unique_ptr<std::thread>> threads;
 	for (int i = 0; i < thread_num; i++)
 	{
-		std::packaged_task<task_result_t()> task([&handler, &thread_wait_sec]{
+		futures.emplace_back(std::async(std::launch::async,
-			auto start = std::chrono::high_resolution_clock::now();
+			[&handler, thread_wait_sec]() {
-			task_result_t res;
+				auto start = std::chrono::high_resolution_clock::now();
-			res.handled = false;
+				task_result_t res;
-			while (!handler.handled())
+				res.handled = false;
-			{
+				while (!handler.handled())
-				if (handler.triggered())
 				{
-					res.handled = handler.handle([&thread_wait_sec]{
+					if (handler.triggered())
-						std::this_thread::sleep_for (std::chrono::seconds(thread_wait_sec));
+					{
-					});
+						res.handled = handler.handle([thread_wait_sec]() {
+							std::this_thread::sleep_for(thread_wait_sec);
+						});
+					}
 				}
-			}
+				auto diff = std::chrono::high_resolution_clock::now() - start;
-			auto diff = std::chrono::high_resolution_clock::now() - start;
+				res.duration_secs = std::chrono::duration_cast<std::chrono::seconds>(diff);
-			res.duration_secs = std::chrono::duration_cast<std::chrono::seconds>(diff).count();
+				return res;
-			return res;
+			}));
-		});
-		futures.push_back(task.get_future());
-		threads.emplace_back();
-		threads[i].reset(new std::thread(std::move(task)));
 	}
 
 	// wait a bit, then trigger the signal handler from the main thread
 	auto total_handled = 0;
 	auto start = std::chrono::high_resolution_clock::now();
-	std::this_thread::sleep_for (std::chrono::seconds(handler_wait_sec));
+	std::this_thread::sleep_for(handler_wait_sec);
 	handler.trigger();
 	for (int i = 0; i < thread_num; i++)
 	{
-		// we need to check that all threads didn't quit before
+		// wait for all threads to finish and get the results from the futures
-		// the handle() function finished executing
-		futures[i].wait();
-		threads[i]->join();
 		auto res = futures[i].get();
 		if (res.handled)
 		{
@@ -92,7 +87,7 @@ TEST(AtomicSignalHandler, handle_once_wait_consistency)
 
 	// check that the total time is consistent with the expectations
 	auto diff = std::chrono::high_resolution_clock::now() - start;
-	auto secs = std::chrono::duration_cast<std::chrono::seconds>(diff).count();
+	auto secs = std::chrono::duration_cast<std::chrono::seconds>(diff);
 	ASSERT_GE(secs, thread_wait_sec + handler_wait_sec);
 
 	// check that only one thread handled the signal

userspace/engine/evttype_index_ruleset.cpp

@@ -199,7 +199,7 @@ void evttype_index_ruleset::add(
 {
 	try
 	{
-		std::shared_ptr<filter_wrapper> wrap(new filter_wrapper());
+		auto wrap = std::make_shared<filter_wrapper>();
 		wrap->rule = rule;
 		wrap->filter = filter;
 		if(rule.source == falco_common::syscall_source)