bunch of refactoring and cleanup

GUI/exception.cpp

@@ -21,7 +21,11 @@ namespace
 
 	LONG WINAPI ExceptionLogger(EXCEPTION_POINTERS* exception)
 	{
-		thread_local static auto _ = std::invoke(std::set_terminate, Terminate);
+		thread_local static auto _ = std::invoke([]
+		{
+			std::set_terminate(Terminate);
+			return 0;
+		});
 
 		MEMORY_BASIC_INFORMATION info = {};
 		VirtualQuery(exception->ExceptionRecord->ExceptionAddress, &info, sizeof(info));
@@ -46,6 +50,7 @@ namespace
 	auto _ = std::invoke([]
 	{
 		AddVectoredExceptionHandler(FALSE, ExceptionLogger);
-		return SetUnhandledExceptionFilter([](auto) -> LONG { Terminate(); });
+		SetUnhandledExceptionFilter([](auto) -> LONG { Terminate(); });
+		return 0;
 	});
 }

GUI/extenwindow.cpp

@@ -35,21 +35,21 @@ namespace
 		if (!module) return;
 		FARPROC callback = GetProcAddress(module, "OnNewSentence");
 		if (!callback) return;
-		LOCK(extenMutex);
+		std::scoped_lock writeLock(extenMutex);
 		extensions[extenName] = (wchar_t*(*)(const wchar_t*, const InfoForExtension*))callback;
 		extenNames.push_back(extenName);
 	}
 
 	void Unload(QString extenName)
 	{
-		LOCK(extenMutex);
+		std::scoped_lock writeLock(extenMutex);
 		extenNames.erase(std::remove(extenNames.begin(), extenNames.end(), extenName), extenNames.end());
 		FreeLibrary(GetModuleHandleW(S(extenName).c_str()));
 	}
 
 	void Reorder(QStringList extenNames)
 	{
-		LOCK(extenMutex);
+		std::scoped_lock writeLock(extenMutex);
 		::extenNames = extenNames;
 	}
 }
@@ -63,7 +63,7 @@ bool DispatchSentenceToExtensions(std::wstring& sentence, std::unordered_map<con
 	InfoForExtension* miscInfoTraverser = &miscInfoLinkedList;
 	for (auto[name, value] : miscInfo) miscInfoTraverser = miscInfoTraverser->next = new InfoForExtension{ name, value, nullptr };
 
-	std::shared_lock sharedLock(extenMutex);
+	std::shared_lock readLock(extenMutex);
 	for (auto extenName : extenNames)
 	{
 		wchar_t* nextBuffer = extensions[extenName](sentenceBuffer, &miscInfoLinkedList);
@@ -100,7 +100,7 @@ void ExtenWindow::Sync()
 {
 	ui->extenList->clear();
 	QAutoFile extenSaveFile(EXTEN_SAVE_FILE, QIODevice::WriteOnly | QIODevice::Truncate);
-	std::shared_lock sharedLock(extenMutex);
+	std::shared_lock readLock(extenMutex);
 	for (auto extenName : extenNames)
 	{
 		ui->extenList->addItem(extenName);

GUI/host/host.cpp

@@ -31,7 +31,7 @@ namespace
 		TextHook GetHook(uint64_t addr)
 		{
 			if (view == nullptr) return {};
-			LOCK(viewMutex);
+			std::scoped_lock lock(viewMutex);
 			auto hooks = (const TextHook*)view;
 			for (int i = 0; i < MAX_HOOK; ++i)
 				if (hooks[i].address == addr) return hooks[i];
@@ -53,14 +53,17 @@ namespace
 		WinMutex viewMutex;
 	};
 
-	ThreadSafe<std::unordered_map<ThreadParam, std::unique_ptr<TextThread>>, std::recursive_mutex> textThreadsByParams;
+	size_t HashThreadParam(ThreadParam tp)
+	{
+		std::hash<int64_t> hash;
+		return hash(hash(tp.processId) + hash(tp.addr) + hash(tp.ctx) + hash(tp.ctx2));
+	}
+	ThreadSafe<std::unordered_map<ThreadParam, std::unique_ptr<TextThread>, Functor<HashThreadParam>>, std::recursive_mutex> textThreadsByParams;
 	ThreadSafe<std::unordered_map<DWORD, ProcessRecord>, std::recursive_mutex> processRecordsByIds;
 
-	ThreadParam CONSOLE{ 0, -1ULL, -1ULL, -1ULL }, CLIPBOARD{ 0, 0, -1ULL, -1ULL };
-
 	void RemoveThreads(std::function<bool(ThreadParam)> removeIf)
 	{
-		std::vector<std::unique_ptr<TextThread>> removedThreads;
+		std::vector<std::unique_ptr<TextThread>> removedThreads; // delay destruction until after lock is released
 		auto[lock, textThreadsByParams] = ::textThreadsByParams.operator->();
 		for (auto it = textThreadsByParams->begin(); it != textThreadsByParams->end(); removeIf(it->first) ? it = textThreadsByParams->erase(it) : ++it)
 			if (removeIf(it->first)) removedThreads.emplace_back(std::move(it->second));
@@ -73,10 +76,11 @@ namespace
 			SECURITY_DESCRIPTOR pipeSD = {};
 			InitializeSecurityDescriptor(&pipeSD, SECURITY_DESCRIPTOR_REVISION);
 			SetSecurityDescriptorDacl(&pipeSD, TRUE, NULL, FALSE); // Allow non-admin processes to connect to pipe created by admin host
-			SECURITY_ATTRIBUTES pipeSA = { sizeof(SECURITY_ATTRIBUTES), &pipeSD, FALSE };
+			SECURITY_ATTRIBUTES pipeSA = { sizeof(pipeSA), &pipeSD, FALSE };
 
-			struct NamedPipeHandleCloser { void operator()(void* h) { DisconnectNamedPipe(h); CloseHandle(h); } };
+
-			AutoHandle<NamedPipeHandleCloser>
+			struct PipeCloser { void operator()(HANDLE h) { DisconnectNamedPipe(h); CloseHandle(h); } };
+			AutoHandle<PipeCloser>
 				hookPipe = CreateNamedPipeW(HOOK_PIPE, PIPE_ACCESS_INBOUND, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, PIPE_UNLIMITED_INSTANCES, 0, PIPE_BUFFER_SIZE, MAXDWORD, &pipeSA),
 				hostPipe = CreateNamedPipeW(HOST_PIPE, PIPE_ACCESS_OUTBOUND, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, PIPE_UNLIMITED_INSTANCES, PIPE_BUFFER_SIZE, 0, MAXDWORD, &pipeSA);
 			ConnectNamedPipe(hookPipe, nullptr);
@@ -84,7 +88,7 @@ namespace
 			BYTE buffer[PIPE_BUFFER_SIZE] = {};
 			DWORD bytesRead, processId;
 			ReadFile(hookPipe, &processId, sizeof(processId), &bytesRead, nullptr);
-			processRecordsByIds->try_emplace(processId, processId, hostPipe);
+			processRecordsByIds->emplace(processId, processId, hostPipe);
 
 			CreatePipe();
 
@@ -122,7 +126,7 @@ namespace
 		SetWindowsHookExW(WH_GETMESSAGE, [](int statusCode, WPARAM wParam, LPARAM lParam)
 		{
 			if (statusCode == HC_ACTION && wParam == PM_REMOVE && ((MSG*)lParam)->message == WM_CLIPBOARDUPDATE)
-				if (auto text = Util::GetClipboardText()) Host::GetThread(CLIPBOARD)->AddSentence(text.value());
+				if (auto text = Util::GetClipboardText()) Host::GetThread(Host::clipboard)->AddSentence(text.value());
 			return CallNextHookEx(NULL, statusCode, wParam, lParam);
 		}, NULL, GetCurrentThreadId());
 	}
@@ -137,9 +141,9 @@ namespace Host
 		TextThread::OnCreate = OnCreate;
 		TextThread::OnDestroy = OnDestroy;
 		TextThread::Output = Output;
-		processRecordsByIds->try_emplace(CONSOLE.processId, CONSOLE.processId, INVALID_HANDLE_VALUE);
+		processRecordsByIds->emplace(console.processId, console.processId, INVALID_HANDLE_VALUE);
-		textThreadsByParams->insert({ CONSOLE, std::make_unique<TextThread>(CONSOLE, HookParam{}, L"Console") });
+		textThreadsByParams->insert({ console, std::make_unique<TextThread>(console, HookParam{}, CONSOLE) });
-		textThreadsByParams->insert({ CLIPBOARD, std::make_unique<TextThread>(CLIPBOARD, HookParam{}, L"Clipboard") });
+		textThreadsByParams->insert({ Host::clipboard, std::make_unique<TextThread>(Host::clipboard, HookParam{}, CLIPBOARD) });
 		StartCapturingClipboard();
 		CreatePipe();
 	}
@@ -169,7 +173,7 @@ namespace Host
 				return false;
 			}
 #endif
-			if (LPVOID remoteData = VirtualAllocEx(process, nullptr, location.size() * 2 + 2, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE))
+			if (LPVOID remoteData = VirtualAllocEx(process, nullptr, (location.size() + 1) * sizeof(wchar_t), MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE))
 			{
 				WriteProcessMemory(process, remoteData, location.c_str(), location.size() * 2 + 2, nullptr);
 				if (AutoHandle<> thread = CreateRemoteThread(process, nullptr, 0, (LPTHREAD_START_ROUTINE)LoadLibraryW, remoteData, 0, nullptr))
@@ -208,6 +212,6 @@ namespace Host
 
 	void AddConsoleOutput(std::wstring text)
 	{
-		GetThread(CONSOLE)->AddSentence(text);
+		GetThread(console)->AddSentence(text);
 	}
 }

GUI/host/host.h

@@ -18,4 +18,6 @@ namespace Host
 	void AddConsoleOutput(std::wstring text);
 
 	inline int defaultCodepage = SHIFT_JIS;
+
+	constexpr ThreadParam console{ 0, -1LL, -1LL, -1LL }, clipboard{ 0, 0, -1LL, -1LL };
 }

GUI/host/textthread.cpp

@@ -27,7 +27,7 @@ void TextThread::AddSentence(const std::wstring& sentence)
 void TextThread::Push(const BYTE* data, int len)
 {
 	if (len < 0) return;
-	LOCK(bufferMutex);
+	std::scoped_lock lock(bufferMutex);
 	if (hp.type & USING_UNICODE) buffer += std::wstring((wchar_t*)data, len / 2);
 	else if (auto converted = Util::StringToWideString(std::string((char*)data, len), hp.codepage ? hp.codepage : Host::defaultCodepage)) buffer += converted.value();
 	else Host::AddConsoleOutput(INVALID_CODEPAGE);
@@ -49,7 +49,7 @@ void TextThread::Flush()
 	for (auto sentence : sentences)
 		if (Output(this, sentence)) storage->append(sentence);
 
-	LOCK(bufferMutex);
+	std::scoped_lock lock(bufferMutex);
 	if (buffer.empty()) return;
 	if (buffer.size() < maxBufferSize && GetTickCount() - lastPushTime < flushDelay) return;
 	AddSentence(buffer);

GUI/host/textthread.h

@@ -31,11 +31,11 @@ private:
 
 	void Flush();
 
-	struct TimerDeleter { void operator()(void* h) { DeleteTimerQueueTimer(NULL, h, INVALID_HANDLE_VALUE); } };
 	std::wstring buffer;
 	std::unordered_set<wchar_t> repeatingChars;
 	std::mutex bufferMutex;
 	DWORD lastPushTime;
 	ThreadSafe<std::vector<std::wstring>> queuedSentences;
+	struct TimerDeleter { void operator()(HANDLE h) { DeleteTimerQueueTimer(NULL, h, INVALID_HANDLE_VALUE); } };
 	AutoHandle<TimerDeleter> timer = NULL; // this needs to be last so it's destructed first
 };

GUI/host/util.cpp

@@ -10,7 +10,7 @@ namespace Util
 		{
 			std::vector<wchar_t> buffer(MAX_PATH);
 			if (GetModuleFileNameExW(process, module, buffer.data(), MAX_PATH)) return buffer.data();
-			else return {};
+			return {};
 		}
 		return {};
 	}
@@ -18,8 +18,8 @@ namespace Util
 	std::optional<std::wstring> GetModuleFilename(HMODULE module)
 	{
 		std::vector<wchar_t> buffer(MAX_PATH);
-		if (::GetModuleFileNameW(module, buffer.data(), MAX_PATH)) return buffer.data();
+		if (GetModuleFileNameW(module, buffer.data(), MAX_PATH)) return buffer.data();
-		else return {};
+		return {};
 	}
 
 	std::optional<std::wstring> GetClipboardText()
@@ -27,22 +27,17 @@ namespace Util
 		if (!IsClipboardFormatAvailable(CF_UNICODETEXT)) return {};
 		if (!OpenClipboard(NULL)) return {};
 
-		if (HANDLE clipboard = GetClipboardData(CF_UNICODETEXT))
+		std::optional<std::wstring> ret;
-		{
+		if (AutoHandle<Functor<GlobalUnlock>> clipboard = GetClipboardData(CF_UNICODETEXT)) ret = (wchar_t*)GlobalLock(clipboard);
-			std::wstring ret = (wchar_t*)GlobalLock(clipboard);
-			GlobalUnlock(clipboard);
 		CloseClipboard();
 		return ret;
 	}
-		CloseClipboard();
-		return {};
-	}
 
 	std::optional<std::wstring> StringToWideString(std::string text, UINT encoding)
 	{
 		std::vector<wchar_t> buffer(text.size() + 1);
 		if (MultiByteToWideChar(encoding, 0, text.c_str(), -1, buffer.data(), buffer.size())) return buffer.data();
-		else return {};
+		return {};
 	}
 
 	bool RemoveRepetition(std::wstring& text)

GUI/mainwindow.cpp

@@ -53,11 +53,11 @@ MainWindow::MainWindow(QWidget *parent) :
 
 	std::thread([]
 	{
+		using InternetHandle = AutoHandle<Functor<WinHttpCloseHandle>>;
 		// Queries GitHub releases API https://developer.github.com/v3/repos/releases/ and checks the last release tag to check if it's the same
-		struct InternetHandleCloser { void operator()(void* h) { WinHttpCloseHandle(h); } };
+		if (InternetHandle internet = WinHttpOpen(L"Mozilla/5.0 Textractor", WINHTTP_ACCESS_TYPE_DEFAULT_PROXY, NULL, NULL, 0))
-		if (AutoHandle<InternetHandleCloser> internet = WinHttpOpen(L"Mozilla/5.0 Textractor", WINHTTP_ACCESS_TYPE_DEFAULT_PROXY, NULL, NULL, 0))
+			if (InternetHandle connection = WinHttpConnect(internet, L"api.github.com", INTERNET_DEFAULT_HTTPS_PORT, 0))
-			if (AutoHandle<InternetHandleCloser> connection = WinHttpConnect(internet, L"api.github.com", INTERNET_DEFAULT_HTTPS_PORT, 0))
+				if (InternetHandle request = WinHttpOpenRequest(connection, L"GET", L"/repos/Artikash/Textractor/releases", NULL, NULL, NULL, WINHTTP_FLAG_SECURE))
-				if (AutoHandle<InternetHandleCloser> request = WinHttpOpenRequest(connection, L"GET", L"/repos/Artikash/Textractor/releases", NULL, NULL, NULL, WINHTTP_FLAG_SECURE))
 					if (WinHttpSendRequest(request, NULL, 0, NULL, 0, 0, NULL))
 					{
 						DWORD bytesRead;

include/const.h

@@ -24,7 +24,7 @@ enum HostNotificationType
 	HOST_NOTIFICATION_RMVHOOK = 2
 };
 
-enum HookParamType : unsigned long
+enum HookParamType : unsigned
 {
 	USING_STRING = 0x1, // type(data) is char* or wchar_t* and has length
 	USING_UNICODE = 0x2, // type(data) is wchar_t or wchar_t*

include/text.h

@@ -22,7 +22,7 @@ u8"OR\r\n"
 u8"Enter hook code\r\n"
 u8"H{A|B|W|S|Q|V}[N][codepage#]data_offset[*deref_offset1][:split_offset[*deref_offset2]]@addr[:module[:func]]\r\n"
 u8"All numbers except codepage in hexadecimal\r\n"
-u8"Default codepage is 932 (Shift-JIS) but this can be changed in settings"
+u8"Default codepage is 932 (Shift-JIS) but this can be changed in settings\r\n"
 u8"A/B: codepage char little/big endian\r\n"
 u8"W: UTF-16 char\r\n"
 u8"S/Q/V: codepage/UTF-16/UTF-8 string\r\n"
@@ -37,6 +37,8 @@ constexpr auto WINDOW = u8"Window";
 constexpr auto DEFAULT_CODEPAGE = u8"Default Codepage";
 constexpr auto FLUSH_DELAY = u8"Flush Delay";
 constexpr auto MAX_BUFFER_SIZE = u8"Max Buffer Size";
+constexpr auto CONSOLE = L"Console";
+constexpr auto CLIPBOARD = L"Clipboard";
 constexpr auto ABOUT = L"Textractor beta v" CURRENT_VERSION L" (project homepage: https://github.com/Artikash/Textractor)\r\n"
 L"Made by me: Artikash (email: akashmozumdar@gmail.com)\r\n"
 L"Please contact me with any problems, feature requests, or questions relating to Textractor\r\n"

include/types.h

@@ -5,6 +5,13 @@
 
 template <typename T> using Array = T[];
 
+template <auto F>
+struct Functor
+{
+	template <typename... Args>
+	auto operator()(Args&&... args) const { return std::invoke(F, std::forward<Args>(args)...); }
+};
+
 template<typename E, typename M = std::mutex>
 class ThreadSafe
 {
@@ -26,8 +33,7 @@ private:
 	M mtx;
 };
 
-struct DefHandleCloser { void operator()(void* h) { CloseHandle(h); } };
+template <typename HandleCloser = Functor<CloseHandle>>
-template <typename HandleCloser = DefHandleCloser>
 class AutoHandle
 {
 public:
@@ -74,16 +80,14 @@ struct HookParam
 
 struct ThreadParam
 {
+	bool operator==(ThreadParam other) const { return processId == other.processId && addr == other.addr && ctx == other.ctx && ctx2 == other.ctx2; }
 	DWORD processId;
 	uint64_t addr;
 	uint64_t ctx; // The context of the hook: by default the first value on stack, usually the return address
 	uint64_t ctx2;  // The subcontext of the hook: 0 by default, generated in a method specific to the hook
 };
-// Artikash 5/31/2018: required for unordered_map to work with struct key
-template <> struct std::hash<ThreadParam> { size_t operator()(ThreadParam tp) const { return std::hash<int64_t>()((tp.processId + tp.addr) ^ (tp.ctx + tp.ctx2)); } };
-static bool operator==(ThreadParam one, ThreadParam two) { return one.processId == two.processId && one.addr == two.addr && one.ctx == two.ctx && one.ctx2 == two.ctx2; }
 
-class WinMutex // Like CMutex but works with lock_guard
+class WinMutex // Like CMutex but works with scoped_lock
 {
 public:
 	WinMutex(std::wstring name) : m(CreateMutexW(nullptr, FALSE, name.c_str())) {}
@@ -114,5 +118,3 @@ struct HookRemovedNotif // From hook
 	int command = HOST_NOTIFICATION_RMVHOOK;
 	uint64_t address;
 };
-
-#define LOCK(mutex) std::lock_guard lock(mutex)

vnrhook/main.cc

@@ -78,11 +78,11 @@ DWORD WINAPI Pipe(LPVOID)
 void TextOutput(ThreadParam tp, BYTE* text, int len)
 {
 	if (len < 0) return;
-	if (len > PIPE_BUFFER_SIZE - sizeof(ThreadParam)) len = PIPE_BUFFER_SIZE - sizeof(ThreadParam);
+	if (len > PIPE_BUFFER_SIZE - sizeof(tp)) len = PIPE_BUFFER_SIZE - sizeof(tp);
 	BYTE buffer[PIPE_BUFFER_SIZE] = {};
 	*(ThreadParam*)buffer = tp;
-	memcpy(buffer + sizeof(ThreadParam), text, len);
+	memcpy(buffer + sizeof(tp), text, len);
-	WriteFile(hookPipe, buffer, sizeof(ThreadParam) + len, &DUMMY, nullptr);
+	WriteFile(hookPipe, buffer, sizeof(tp) + len, &DUMMY, nullptr);
 }
 
 void ConsoleOutput(LPCSTR text, ...)

vnrhook/texthook.cc

@@ -100,7 +100,7 @@ void SetTrigger()
 
 bool TextHook::Insert(HookParam h, DWORD set_flag)
 {
-	LOCK(*viewMutex);
+	std::scoped_lock lock(*viewMutex);
 	hp = h;
 	address = hp.address;
 	hp.type |= set_flag;
@@ -280,7 +280,7 @@ void TextHook::RemoveReadCode()
 
 void TextHook::Clear()
 {
-	LOCK(*viewMutex);
+	std::scoped_lock lock(*viewMutex);
 	ConsoleOutput(REMOVING_HOOK, hp.name);
 	if (hp.type & DIRECT_READ) RemoveReadCode();
 	else RemoveHookCode();