Merge pull request #19 from mandiant/destructors

Destructors
mandiant · Aug 18, 2023 · d467aba · d467aba
2 parents b7c0ace + 68b110c
commit d467aba
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 22 deletions.
diff --git a/C/STrace/EtwLogger.cpp b/C/STrace/EtwLogger.cpp
@@ -132,9 +132,9 @@ EtwProvider::EtwProvider(EtwProvider&& other)
 
 EtwProvider& EtwProvider::operator=(EtwProvider&& other)
 {
-	m_guid = move(other.m_guid);
-	m_regHandle = move(other.m_regHandle);
-	m_providerMetadataDesc = move(other.m_providerMetadataDesc);
+	m_guid = other.m_guid;
+	m_regHandle = other.m_regHandle;
+	m_providerMetadataDesc = other.m_providerMetadataDesc;
 	m_events = move(other.m_events);
 
 	return *this;
@@ -176,12 +176,8 @@ NTSTATUS EtwProvider::Initialize(const char* providerName)
 	return STATUS_SUCCESS;
 }
 
-void EtwProvider::Destruct()
+EtwProvider::~EtwProvider()
 {
-	for (auto i = 0; i < m_events.len(); i++)
-	{
-		m_events[i].Destruct();
-	}
 	m_events.Destruct();
 
 	if (m_regHandle != 0)
@@ -250,6 +246,7 @@ NTSTATUS EtwProvider::WriteEvent(const char* eventName, uint8_t eventLevel, uint
 		dataDescriptors[i + 2] = CreateTraceProperty(fieldType, GetFieldAddress(fieldType, fieldValue));
 		if (dataDescriptors[i + 2].Ptr == NULL)
 		{
+			va_end(args);
 			return STATUS_UNSUCCESSFUL;
 		}
 	}
@@ -508,7 +505,7 @@ NTSTATUS EtwProviderEvent::Initialize(const char* eventName, int numberOfFields,
 	return STATUS_SUCCESS;
 }
 
-void EtwProviderEvent::Destruct()
+EtwProviderEvent::~EtwProviderEvent()
 {
 	if (m_eventMetadataDesc.Ptr != NULL)
 	{
@@ -566,26 +563,29 @@ NTSTATUS EtwTrace(
 	auto etwProvider = FindProvider(providerGuid);
 	if (etwProvider == NULL)
 	{
-		detail::EtwProvider newEtwProvider{ providerGuid };
-		const auto status = newEtwProvider.Initialize(providerName);
+		// emplace so we don't call dtor on a temporary
+		g_ProviderCache.emplace_back(providerGuid);
+		etwProvider = &g_ProviderCache.back();
+
+		// initialize the class here
+		const auto status = etwProvider->Initialize(providerName);
 		if (status != STATUS_SUCCESS)
 		{
+			// oof failed, remove paritally init'd object
+			g_ProviderCache.pop_back();
 			return status;
 		}
-
-		g_ProviderCache.push_back(move(newEtwProvider));
-		etwProvider = &g_ProviderCache.back();
 	}
 
 	// Add the event to the provider.
 	va_list args;
 	va_start(args, numberOfFields);
 	auto status = etwProvider->AddEvent(eventName, numberOfFields, args);
+	va_end(args);
 	if (status != STATUS_SUCCESS)
 	{
 		return status;
 	}
-	va_end(args);
 
 	// Write the event.
 	va_list args2;

diff --git a/C/STrace/EtwLogger.h b/C/STrace/EtwLogger.h
@@ -11,7 +11,7 @@ class EtwProviderEvent
 {
 public:
 	EtwProviderEvent();
-	void Destruct();
+	~EtwProviderEvent();
 
 	NTSTATUS Initialize(const char* eventName, int numberOfFields, va_list fields);
 
@@ -34,10 +34,11 @@ class EtwProvider
 public:
 	EtwProvider(LPCGUID providerGuid);
 	EtwProvider(const EtwProvider& other) = delete;
-	EtwProvider(EtwProvider&& other);
 	EtwProvider& operator=(const EtwProvider& other) = delete;
+	EtwProvider(EtwProvider&& other);
 	EtwProvider& operator=(EtwProvider&& other);
-	void Destruct();
+
+	~EtwProvider();
 
 	NTSTATUS Initialize(const char* name);
 	NTSTATUS AddEvent(const char* eventName, int numberOfFields, va_list fields);

diff --git a/C/STrace/driver.cpp b/C/STrace/driver.cpp
@@ -539,10 +539,6 @@ Return Value:
     //
     // Unregister any registered ETW providers.
     //
-    for (auto i = 0; i < g_ProviderCache.len(); i++)
-    {
-        g_ProviderCache[i].Destruct();
-    }
     g_ProviderCache.Destruct();
 
     //

diff --git a/C/STrace/vector.h b/C/STrace/vector.h
@@ -68,6 +68,11 @@ class MyVector {
 		data[size++] = move(item);
 	}
 
+	template<typename... Args>
+	void emplace_back(Args&&... args) {
+		push_back(move(T(args...)));
+	}
+
 	void pop_back() {
 		data[size].T::~T();
 		size--;