From 1b5e2d110ccdfabd96e7d71f8bd1dd14fb17edce Mon Sep 17 00:00:00 2001
From: Dan Goodliffe <dan.goodliffe@octal.co.uk>
Date: Thu, 14 Feb 2019 14:40:11 +0000
Subject: Enable (most) high integrity checks (not for tests)

---
 Jamroot.jam                        |  4 +++-
 libadhocutil/buffer.cpp            |  7 ++++++-
 libadhocutil/buffer.h              |  2 +-
 libadhocutil/curlMultiHandle.cpp   |  7 +++++--
 libadhocutil/fileUtils.cpp         | 24 +++++++++++++++---------
 libadhocutil/fileUtils.h           | 16 ++++++++--------
 libadhocutil/lexer-regex.cpp       |  9 ++++++++-
 libadhocutil/lexer.cpp             |  2 +-
 libadhocutil/lexer.h               |  2 +-
 libadhocutil/plugins.cpp           |  7 +++----
 libadhocutil/plugins.h             |  2 +-
 libadhocutil/processPipes.cpp      | 26 ++++++++++++++++----------
 libadhocutil/runtimeContext.cpp    | 18 ++++++------------
 libadhocutil/runtimeContext.h      |  5 +++--
 libadhocutil/scopeExit.cpp         | 32 ++++++++++++++++++++++++--------
 libadhocutil/unittests/Jamfile.jam |  1 +
 16 files changed, 102 insertions(+), 62 deletions(-)

diff --git a/Jamroot.jam b/Jamroot.jam
index 1e66cc8..0f2097c 100644
--- a/Jamroot.jam
+++ b/Jamroot.jam
@@ -23,7 +23,9 @@ project
 			<toolset>tidy:<checkxx>misc-*
 			<toolset>tidy:<checkxx>modernize-*
 #<toolset>tidy:<checkxx>cppcoreguidelines-*
-#<toolset>tidy:<checkxx>hicpp-*
+			<toolset>tidy:<checkxx>hicpp-*
+			<toolset>tidy:<xcheckxx>hicpp-vararg
+			<toolset>tidy:<xcheckxx>hicpp-signed-bitwise
 			<toolset>tidy:<checkxx>performance-*
 	;
 
diff --git a/libadhocutil/buffer.cpp b/libadhocutil/buffer.cpp
index 2741c4a..cb071a2 100644
--- a/libadhocutil/buffer.cpp
+++ b/libadhocutil/buffer.cpp
@@ -41,6 +41,7 @@ Buffer::CStringFragment::CStringFragment(char * b, CStringHandling h, size_t l)
 Buffer::CStringFragment::~CStringFragment()
 {
 	if (handling != Use) { // Copy or Free
+		// NOLINTNEXTLINE(hicpp-no-malloc)
 		free(const_cast<char *>(buf));
 	}
 }
@@ -73,7 +74,7 @@ Buffer::CStringFragment::str() const
 // Std::String Fragment
 //
 
-Buffer::StringFragment::StringFragment(const std::string str) :
+Buffer::StringFragment::StringFragment(std::string str) :
 	buf(std::move(str))
 {
 }
@@ -166,8 +167,11 @@ Buffer &
 Buffer::appendf(const char * fmt, ...)
 {
 	va_list v;
+	// NOLINTNEXTLINE(hicpp-no-array-decay)
 	va_start(v, fmt);
+	// NOLINTNEXTLINE(hicpp-no-array-decay)
 	vappendf(fmt, v);
+	// NOLINTNEXTLINE(hicpp-no-array-decay)
 	va_end(v);
 	return *this;
 }
@@ -181,6 +185,7 @@ Buffer::vappendf(const char * fmt, va_list args)
 		content.push_back(std::make_shared<CStringFragment>(frag, Free, len));
 	}
 	else {
+		// NOLINTNEXTLINE(hicpp-no-malloc)
 		free(frag);
 	}
 	return *this;
diff --git a/libadhocutil/buffer.h b/libadhocutil/buffer.h
index afbb100..ca02bbb 100644
--- a/libadhocutil/buffer.h
+++ b/libadhocutil/buffer.h
@@ -145,7 +145,7 @@ class DLL_PUBLIC Buffer {
 
 		class DLL_PRIVATE StringFragment : public FragmentBase {
 			public:
-				StringFragment(const std::string);
+				StringFragment(std::string);
 
 				size_t length() const;
 				char operator[](size_t) const;
diff --git a/libadhocutil/curlMultiHandle.cpp b/libadhocutil/curlMultiHandle.cpp
index 381979f..ac40b49 100644
--- a/libadhocutil/curlMultiHandle.cpp
+++ b/libadhocutil/curlMultiHandle.cpp
@@ -9,7 +9,7 @@ namespace Net {
 
 class RunningCurl : public CurlStreamSource {
 	public:
-		RunningCurl(const std::string & url, const std::function<void(std::istream &)> c) :
+		RunningCurl(const std::string & url, std::function<void(std::istream &)> c) :
 			CurlStreamSource(url),
 			consumer(std::move(c))
 		{
@@ -59,7 +59,7 @@ CurlMultiHandle::performAll()
 		CURLMcode code;
 		int act = running.size();
 		while (act) {
-			while ((code = curl_multi_perform(curlm, &act)) == CURLM_CALL_MULTI_PERFORM) ;
+			while ((code = curl_multi_perform(curlm, &act)) == CURLM_CALL_MULTI_PERFORM) {}
 			// Has anything finished
 			CURLMsg * msg;
 			int msgs = 0;
@@ -80,8 +80,11 @@ CurlMultiHandle::performAll()
 			fd_set r, w, e;
 			int maxfd = 0;
 			struct timeval to = { 0, 100000 };
+			// NOLINTNEXTLINE(hicpp-no-assembler)
 			FD_ZERO(&r);
+			// NOLINTNEXTLINE(hicpp-no-assembler)
 			FD_ZERO(&w);
+			// NOLINTNEXTLINE(hicpp-no-assembler)
 			FD_ZERO(&e);
 			curl_multi_fdset(curlm, &r, &w, &e, &maxfd);
 			select(act, &r, &w, &e, &to);
diff --git a/libadhocutil/fileUtils.cpp b/libadhocutil/fileUtils.cpp
index 540990a..4e7b809 100644
--- a/libadhocutil/fileUtils.cpp
+++ b/libadhocutil/fileUtils.cpp
@@ -12,16 +12,18 @@ namespace AdHoc {
 		std::filesystem::path operator/(const std::filesystem::path & p, unsigned int n)
 		{
 			auto pp = p.begin();
-			while (n--) ++pp;
+			while (n--) {
+				++pp;
+			}
 			return *pp;
 		}
 
-		FileHandle::FileHandle(int d) :
+		FileHandle::FileHandle(int d) noexcept :
 			fh(d)
 		{
 		}
 
-		FileHandle::FileHandle(FileHandle && o) :
+		FileHandle::FileHandle(FileHandle && o) noexcept :
 			fh(o.fh)
 		{
 			const_cast<int &>(o.fh) = -1;
@@ -43,38 +45,42 @@ namespace AdHoc {
 			}
 		}
 
-		FileHandle::~FileHandle()
+		FileHandle::~FileHandle() noexcept
 		{
 			if (fh >= 0) {
+				// NOLINTNEXTLINE(hicpp-no-array-decay)
 				BOOST_VERIFY(close(fh) == 0);
 			}
 		}
 
-		FileHandle::operator int() const
+		FileHandle::operator int() const noexcept
 		{
 			return fh;
 		}
 
 		FileHandleStat::FileHandleStat(int fd) :
-			FileHandle(fd)
+			FileHandle(fd),
+			st({})
 		{
 			refreshStat();
 		}
 
 		FileHandleStat::FileHandleStat(const std::filesystem::path & path, int flags) :
-			FileHandle(path, flags)
+			FileHandle(path, flags),
+			st({})
 		{
 			refreshStat(path);
 		}
 
 		FileHandleStat::FileHandleStat(const std::filesystem::path & path, int flags, int mode) :
-			FileHandle(path, flags, mode)
+			FileHandle(path, flags, mode),
+			st({})
 		{
 			refreshStat(path);
 		}
 
 		const struct stat &
-		FileHandleStat::getStat() const
+		FileHandleStat::getStat() const noexcept
 		{
 			return st;
 		}
diff --git a/libadhocutil/fileUtils.h b/libadhocutil/fileUtils.h
index 139dbb5..c922bb9 100644
--- a/libadhocutil/fileUtils.h
+++ b/libadhocutil/fileUtils.h
@@ -25,13 +25,13 @@ namespace AdHoc {
 				/**
 				 * Move constructor.
 				 */
-				FileHandle(FileHandle &&);
+				FileHandle(FileHandle &&) noexcept;
 
 				/**
 				 * Construct from an existing file descriptor.
 				 * @param fd An open file descriptor.
 				 */
-				FileHandle(int fd);
+				FileHandle(int fd) noexcept;
 
 				/**
 				 * Open a new file handle.
@@ -48,7 +48,7 @@ namespace AdHoc {
 				 */
 				FileHandle(const std::filesystem::path & path, int flags, int mode);
 
-				virtual ~FileHandle();
+				virtual ~FileHandle() noexcept;
 
 				FileHandle(const FileHandle &) = delete;
 				void operator=(const FileHandle &) = delete;
@@ -57,7 +57,7 @@ namespace AdHoc {
 				 * Implicit conversion back to raw Unix file descriptor.
 				 * @return The container file descriptor.
 				 */
-				operator int() const;
+				operator int() const noexcept;
 
 				/// The file handle.
 				const int fh;
@@ -98,7 +98,7 @@ namespace AdHoc {
 				 * Get the stat structure.
 				 * @return The stat structure.
 				 */
-				const struct stat & getStat() const;
+				const struct stat & getStat() const noexcept;
 
 				/**
 				 * Refresh and return the stat structure.
@@ -163,9 +163,9 @@ namespace AdHoc {
 #endif
 
 			private:
-				DLL_PUBLIC void * setupMapInt(int flags) const;
-				DLL_PUBLIC void * setupMap(int flags) const;
-				DLL_PUBLIC void * setupMap(const std::filesystem::path & path, int flags) const;
+				DLL_PRIVATE void * setupMapInt(int flags) const;
+				DLL_PRIVATE void * setupMap(int flags) const;
+				DLL_PRIVATE void * setupMap(const std::filesystem::path & path, int flags) const;
 		};
 	}
 }
diff --git a/libadhocutil/lexer-regex.cpp b/libadhocutil/lexer-regex.cpp
index 3765285..833210d 100644
--- a/libadhocutil/lexer-regex.cpp
+++ b/libadhocutil/lexer-regex.cpp
@@ -7,7 +7,8 @@ namespace AdHoc {
 				Regex(const Glib::ustring & pattern, GRegexCompileFlags compile, GRegexMatchFlags match) :
 					err(nullptr),
 					regex(g_regex_new(pattern.c_str(), compile, match, &err)),
-					info(nullptr)
+					info(nullptr),
+					str(nullptr)
 				{
 					if (!regex) {
 						auto msg = std::string("Failed to create GRegex: ") + err->message;
@@ -16,6 +17,9 @@ namespace AdHoc {
 					}
 				}
 
+				Regex(const Regex &) = delete;
+				Regex(const Regex &&) = delete;
+
 				~Regex() override
 				{
 					if (err) {
@@ -27,6 +31,9 @@ namespace AdHoc {
 					g_regex_unref(regex);
 				}
 
+				void operator=(const Regex &) = delete;
+				void operator=(const Regex &&) = delete;
+
 				bool matches(const gchar * string, size_t length, size_t position) const override
 				{
 					if (info) {
diff --git a/libadhocutil/lexer.cpp b/libadhocutil/lexer.cpp
index 6c6505f..af339eb 100644
--- a/libadhocutil/lexer.cpp
+++ b/libadhocutil/lexer.cpp
@@ -6,7 +6,7 @@ namespace AdHoc {
 
 	Lexer::Lexer() = default;
 
-	Lexer::Lexer(const Rules r) : rules(std::move(r))
+	Lexer::Lexer(Rules r) : rules(std::move(r))
 	{
 	}
 
diff --git a/libadhocutil/lexer.h b/libadhocutil/lexer.h
index 21c8782..379904d 100644
--- a/libadhocutil/lexer.h
+++ b/libadhocutil/lexer.h
@@ -78,7 +78,7 @@ namespace AdHoc {
 			/// Default constructor (empty rule set)
 			Lexer();
 			/// Construct with an initial set of rules.
-			Lexer(const Rules);
+			Lexer(Rules);
 			/// The lexer's current rule set.
 			Rules rules;
 
diff --git a/libadhocutil/plugins.cpp b/libadhocutil/plugins.cpp
index 34ccae2..9c13bf4 100644
--- a/libadhocutil/plugins.cpp
+++ b/libadhocutil/plugins.cpp
@@ -20,9 +20,8 @@ namespace std {
 	std::ostream &
 	operator<<(std::ostream & s, const std::type_info & t)
 	{
-		char * buf = __cxxabiv1::__cxa_demangle(t.name(), nullptr, nullptr, nullptr);
-		s << buf;
-		free(buf);
+		auto buf = std::unique_ptr<char, decltype(&std::free)>(__cxxabiv1::__cxa_demangle(t.name(), nullptr, nullptr, nullptr), std::free);
+		s << buf.get();
 		return s;
 	}
 }
@@ -46,7 +45,7 @@ namespace AdHoc {
 	}
 
 	AdHocFormatter(DuplicatePluginExceptionMsg, "Duplicate plugin %? for type %? at %?:%?, originally from %?:%?");
-	DuplicatePluginException::DuplicatePluginException(PluginPtr p1, PluginPtr p2) :
+	DuplicatePluginException::DuplicatePluginException(const PluginPtr & p1, const PluginPtr & p2) :
 		std::runtime_error(DuplicatePluginExceptionMsg::get(
 					p1->name, p1->type(), p2->filename, p2->lineno, p1->filename, p1->lineno))
 	{
diff --git a/libadhocutil/plugins.h b/libadhocutil/plugins.h
index d9ed4a3..4ab4385 100644
--- a/libadhocutil/plugins.h
+++ b/libadhocutil/plugins.h
@@ -57,7 +57,7 @@ namespace AdHoc {
 	class DuplicatePluginException : public std::runtime_error {
 		public:
 			/// Constructor taking the original and offending plugin.
-			DuplicatePluginException(PluginPtr p1, PluginPtr p2);
+			DuplicatePluginException(const PluginPtr & p1, const PluginPtr & p2);
 	};
 
 	/// Thrown when a resolver function is added a second time.
diff --git a/libadhocutil/processPipes.cpp b/libadhocutil/processPipes.cpp
index 4973f9b..f968fab 100644
--- a/libadhocutil/processPipes.cpp
+++ b/libadhocutil/processPipes.cpp
@@ -31,16 +31,16 @@ ProcessPipes::ProcessPipes(const std::vector<std::string> & args, bool i, bool o
 	if (args.empty()) {
 		throw std::invalid_argument("args is empty");
 	}
-	int ipipes[2], opipes[2], epipes[2];
+	std::array<int, 2> ipipes { -1, -1 }, opipes { -1, -1 }, epipes { -1, -1 };
 	ScopeExit tidyUp;
 	if (i) {
-		pipe(ipipes, tidyUp);
+		pipe(ipipes.data(), tidyUp);
 	}
 	if (o) {
-		pipe(opipes, tidyUp);
+		pipe(opipes.data(), tidyUp);
 	}
 	if (e) {
-		pipe(epipes, tidyUp);
+		pipe(epipes.data(), tidyUp);
 	}
 	switch (child = fork()) {
 		case -1: // fail
@@ -79,8 +79,8 @@ ProcessPipes::ProcessPipes(const std::vector<std::string> & args, bool i, bool o
 				*w++ = strdup(p.c_str());
 			}
 			*w = nullptr;
-			if (*buf) {
-				execv(buf[0], buf);
+			if (buf[0]) {
+				execv(buf[0], (char * const *)buf);
 			}
 			abort();
 			break;
@@ -89,9 +89,15 @@ ProcessPipes::ProcessPipes(const std::vector<std::string> & args, bool i, bool o
 
 ProcessPipes::~ProcessPipes()
 {
-	if (in) close(in);
-	if (out) close(out);
-	if (error) close(error);
+	if (in >= 0) {
+		close(in);
+	}
+	if (out >= 0) {
+		close(out);
+	}
+	if (error >= 0) {
+		close(error);
+	}
 }
 
 int
@@ -121,7 +127,7 @@ ProcessPipes::pid() const
 void
 ProcessPipes::closeAllOpenFiles()
 {
-	rlimit lim;
+	rlimit lim { };
 	getrlimit(RLIMIT_NOFILE, &lim);
 	std::vector<struct pollfd> fds;
 	fds.reserve(lim.rlim_max);
diff --git a/libadhocutil/runtimeContext.cpp b/libadhocutil/runtimeContext.cpp
index 7c911a5..8d3aeea 100644
--- a/libadhocutil/runtimeContext.cpp
+++ b/libadhocutil/runtimeContext.cpp
@@ -7,23 +7,21 @@ namespace AdHoc {
 namespace System {
 
 RuntimeContext::RuntimeContext(size_t stacksize) :
+	stack(stacksize),
+	ctxInitial({}),
+	ctxCallback({}),
 	completed(false),
 	swapped(false)
 {
-	stack = malloc(stacksize);
-	if (getcontext(&ctxCallback) == -1)
+	if (getcontext(&ctxCallback) == -1) {
 		throw SystemException("getcontext(3) failed", strerror(errno), errno);
-	ctxCallback.uc_stack.ss_sp = stack;
+	}
+	ctxCallback.uc_stack.ss_sp = &stack.front();
 	ctxCallback.uc_stack.ss_size = stacksize;
 	ctxCallback.uc_link = &ctxInitial;
 	makecontext(&ctxCallback, (void (*)())&RuntimeContext::callbackWrapper, 1, this);
 }
 
-RuntimeContext::~RuntimeContext()
-{
-	free(stack);
-}
-
 void
 RuntimeContext::swapContext()
 {
@@ -34,10 +32,6 @@ RuntimeContext::swapContext()
 	else {
 		if (!completed) {
 			swapcontext(&ctxCallback, &ctxInitial);
-			if (completed) {
-				free(stack);
-				stack = nullptr;
-			}
 		}
 	}
 }
diff --git a/libadhocutil/runtimeContext.h b/libadhocutil/runtimeContext.h
index 8c87263..f7c04cf 100644
--- a/libadhocutil/runtimeContext.h
+++ b/libadhocutil/runtimeContext.h
@@ -4,6 +4,7 @@
 #include <stdlib.h>
 #include <ucontext.h>
 #include "visibility.h"
+#include <vector>
 
 namespace AdHoc {
 namespace System {
@@ -19,7 +20,7 @@ class DLL_PUBLIC RuntimeContext {
 		 * @param stacksize The size in bytes of the new stack.
 		 */
 		RuntimeContext(size_t stacksize = 16384);
-		virtual ~RuntimeContext() = 0;
+		virtual ~RuntimeContext() = default;
 
 		/** Swap to/from the contained stack. */
 		void swapContext();
@@ -34,7 +35,7 @@ class DLL_PUBLIC RuntimeContext {
 	private:
 		DLL_PRIVATE static void callbackWrapper(RuntimeContext * rc);
 
-		void * stack;
+		std::vector<char> stack;
 		ucontext_t ctxInitial;
 		ucontext_t ctxCallback;
 		bool completed;
diff --git a/libadhocutil/scopeExit.cpp b/libadhocutil/scopeExit.cpp
index fc48219..0163aad 100644
--- a/libadhocutil/scopeExit.cpp
+++ b/libadhocutil/scopeExit.cpp
@@ -6,22 +6,38 @@ ScopeExit::ScopeExit() = default;
 
 ScopeExit::ScopeExit(const Event & onexitpre, const Event & onsuccess, const Event & onfailure, const Event & onexitpost)
 {
-	if (onexitpre) onExitPre.push_back(onexitpre);
-	if (onsuccess) onSuccess.push_back(onsuccess);
-	if (onfailure) onFailure.push_back(onfailure);
-	if (onexitpost) onExitPost.push_back(onexitpost);
+	if (onexitpre) {
+		onExitPre.push_back(onexitpre);
+	}
+	if (onsuccess) {
+		onSuccess.push_back(onsuccess);
+	}
+	if (onfailure) {
+		onFailure.push_back(onfailure);
+	}
+	if (onexitpost) {
+		onExitPost.push_back(onexitpost);
+	}
 }
 
 ScopeExit::~ScopeExit()
 {
-	for(const auto & e : onExitPre) e();
+	for(const auto & e : onExitPre) {
+		e();
+	}
 	if (std::uncaught_exceptions()) {
-		for(const auto & e : onFailure) e();
+		for(const auto & e : onFailure) {
+			e();
+		}
 	}
 	else {
-		for(const auto & e : onSuccess) e();
+		for(const auto & e : onSuccess) {
+			e();
+		}
+	}
+	for(const auto & e : onExitPost) {
+		e();
 	}
-	for(const auto & e : onExitPost) e();
 }
 
 }
diff --git a/libadhocutil/unittests/Jamfile.jam b/libadhocutil/unittests/Jamfile.jam
index b9f5c60..8660a87 100644
--- a/libadhocutil/unittests/Jamfile.jam
+++ b/libadhocutil/unittests/Jamfile.jam
@@ -9,6 +9,7 @@ generators.register-standard xxd.h : TEXT : H ;
 
 project
 	: requirements
+			<toolset>tidy:<xcheckxx>hicpp-*
 			<toolset>tidy:<xcheckxx>performance-*
 	;
 
-- 
cgit v1.2.3