Merge pull request #10609 from Icinga/fix-misc-compiler-warnings

Fix misc compiler warnings

lib/base/application.hpp

@@ -119,7 +119,7 @@ protected:
 
 	virtual void OnShutdown();
 
-	void ValidateName(const Lazy<String>& lvalue, const ValidationUtils& utils) final;
+	void ValidateName(const Lazy<String>& lvalue, const ValidationUtils& utils) override final;
 
 private:
 	static Application::Ptr m_Instance; /**< The application instance. */

lib/base/debug.hpp

@@ -8,42 +8,17 @@
 #ifndef I2_DEBUG
 #	define ASSERT(expr) ((void)0)
 #else /* I2_DEBUG */
-#	define ASSERT(expr) ((expr) ? 0 : icinga_assert_fail(#expr, __FILE__, __LINE__))
+#	define ASSERT(expr) ((expr) ? void(0) : icinga_assert_fail(#expr, __FILE__, __LINE__))
 #endif /* I2_DEBUG */
 
-#define VERIFY(expr) ((expr) ? 0 : icinga_assert_fail(#expr, __FILE__, __LINE__))
+#define VERIFY(expr) ((expr) ? void(0) : icinga_assert_fail(#expr, __FILE__, __LINE__))
 
-#ifdef _MSC_VER
+#define ABORT(expr) icinga_assert_fail(#expr, __FILE__, __LINE__)
-#	define NORETURNPRE __declspec(noreturn)
-#else /* _MSC_VER */
-#	define NORETURNPRE
-#endif /* _MSC_VER */
 
-#ifdef __GNUC__
+[[noreturn]] inline void icinga_assert_fail(const char *expr, const char *file, int line) noexcept(true)
-#	define NORETURNPOST __attribute__((noreturn))
-#else /* __GNUC__ */
-#	define NORETURNPOST
-#endif /* __GNUC__ */
-
-NORETURNPRE int icinga_assert_fail(const char *expr, const char *file, int line) NORETURNPOST;
-
-#ifdef _MSC_VER
-#	pragma warning( push )
-#	pragma warning( disable : 4646 ) /* function declared with __declspec(noreturn) has non-void return type */
-#endif /* _MSC_VER */
-
-inline int icinga_assert_fail(const char *expr, const char *file, int line)
 {
 	fprintf(stderr, "%s:%d: assertion failed: %s\n", file, line, expr);
 	std::abort();
-
-#if !defined(__GNUC__) && !defined(_MSC_VER)
-	return 0;
-#endif /* !defined(__GNUC__) && !defined(_MSC_VER) */
 }
 
-#ifdef _MSC_VER
-#	pragma warning( pop )
-#endif /* _MSC_VER */
-
 #endif /* DEBUG_H */

lib/base/exception.hpp

@@ -35,7 +35,7 @@ public:
 	ScriptError(String message);
 	ScriptError(String message, DebugInfo di, bool incompleteExpr = false);
 
-	const char *what(void) const throw() final;
+	const char* what() const noexcept override final;
 
 	DebugInfo GetDebugInfo() const;
 	bool IsIncompleteExpression() const;
@@ -59,7 +59,7 @@ public:
 	ValidationError(const ConfigObject::Ptr& object, const std::vector<String>& attributePath, const String& message);
 	~ValidationError() throw() override;
 
-	const char *what() const throw() override;
+	const char *what() const noexcept override;
 
 	ConfigObject::Ptr GetObject() const;
 	std::vector<String> GetAttributePath() const;
@@ -118,7 +118,7 @@ String DiagnosticInformation(const boost::exception_ptr& eptr, bool verbose = tr
 
 class posix_error : virtual public std::exception, virtual public boost::exception {
 public:
-	const char* what() const noexcept final;
+	const char* what() const noexcept override final;
 
 private:
 	mutable String m_Message;
@@ -153,7 +153,7 @@ public:
 
 	~invalid_downtime_removal_error() noexcept override;
 
-	const char *what() const noexcept final;
+	const char* what() const noexcept override final;
 
 private:
 	String m_Message;

lib/base/io-engine.cpp

@@ -101,9 +101,7 @@ IoEngine::~IoEngine()
 		boost::asio::post(m_IoContext, []() {
 			throw TerminateIoThread();
 		});
-	}
 
-	for (auto& thread : m_Threads) {
 		thread.join();
 	}
 }

lib/base/logger.hpp

@@ -88,7 +88,7 @@ public:
 	}
 
 	void SetSeverity(const String& value, bool suppress_events = false, const Value& cookie = Empty) override;
-	void ValidateSeverity(const Lazy<String>& lvalue, const ValidationUtils& utils) final;
+	void ValidateSeverity(const Lazy<String>& lvalue, const ValidationUtils& utils) override final;
 
 protected:
 	void Start(bool runtimeCreated) override;

lib/base/netstring.cpp

@@ -167,7 +167,7 @@ String NetString::ReadStringFromStream(const Shared<AsioTlsStream>::Ptr& stream,
 		}
 	}
 
-	if (maxMessageLength >= 0 && len > maxMessageLength) {
+	if (maxMessageLength >= 0 && len > static_cast<std::size_t>(maxMessageLength)) {
 		std::stringstream errorMessage;
 		errorMessage << "Max data length exceeded: " << (maxMessageLength / 1024) << " KB";
 
@@ -246,7 +246,7 @@ String NetString::ReadStringFromStream(const Shared<AsioTlsStream>::Ptr& stream,
 		}
 	}
 
-	if (maxMessageLength >= 0 && len > maxMessageLength) {
+	if (maxMessageLength >= 0 && len > static_cast<std::size_t>(maxMessageLength)) {
 		std::stringstream errorMessage;
 		errorMessage << "Max data length exceeded: " << (maxMessageLength / 1024) << " KB";
 

lib/base/object-packer.cpp

@@ -239,8 +239,8 @@ static void PackAny(const Value& value, std::string& builder)
  */
 String icinga::PackObject(const Value& value)
 {
-	std::string builder;
+	String builder;
-	PackAny(value, builder);
+	PackAny(value, builder.GetData());
 
 	return builder;
 }

lib/cli/clicommand.cpp

@@ -228,20 +228,16 @@ void CLICommand::ShowCommands(int argc, char **argv, po::options_description *vi
 	typedef std::map<std::vector<String>, CLICommand::Ptr>::value_type CLIKeyValue;
 
 	std::vector<String> best_match;
-	int arg_begin = 0;
 	CLICommand::Ptr command;
 
 	for (const CLIKeyValue& kv : GetRegistry()) {
 		const std::vector<String>& vname = kv.first;
 
-		arg_begin = 0;
-
 		std::vector<String>::size_type i;
 		int k;
 		for (i = 0, k = 1; i < vname.size() && k < argc; i++, k++) {
 			if (strcmp(argv[k], "--no-stack-rlimit") == 0 || strcmp(argv[k], "--autocomplete") == 0 || strcmp(argv[k], "--scm") == 0) {
 				i--;
-				arg_begin++;
 				continue;
 			}
 

lib/config/configitem.cpp

@@ -444,7 +444,9 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue
 		<< "Committing " << total << " new items.";
 #endif /* I2_DEBUG */
 
+#ifdef I2_DEBUG
 	int itemsCount {0};
+#endif /* I2_DEBUG */
 
 	for (auto& type : Type::GetConfigTypesSortedByLoadDependencies()) {
 		std::atomic<int> committed_items(0);
@@ -475,9 +477,9 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue
 			}
 		}
 
+#ifdef I2_DEBUG
 		itemsCount += committed_items;
 
-#ifdef I2_DEBUG
 		if (committed_items > 0)
 			Log(LogDebug, "configitem")
 				<< "Committed " << committed_items << " items of type '" << type->GetName() << "'.";

lib/config/expression.cpp

@@ -554,7 +554,7 @@ ExpressionResult GetScopeExpression::DoEvaluate(ScriptFrame& frame, DebugHint *d
 	else if (m_ScopeSpec == ScopeGlobal)
 		return frame.GetGlobals();
 	else
-		VERIFY(!"Invalid scope.");
+		ABORT("Invalid scope.");
 }
 
 static inline

lib/icinga/checkable-dependency.cpp

@@ -51,7 +51,7 @@ std::vector<DependencyGroup::Ptr> Checkable::GetDependencyGroups() const
 static std::variant<Checkable*, String> GetDependencyGroupKey(const Dependency::Ptr& dependency)
 {
 	if (auto redundancyGroup(dependency->GetRedundancyGroup()); !redundancyGroup.IsEmpty()) {
-		return std::move(redundancyGroup);
+		return redundancyGroup;
 	}
 
 	return dependency->GetParent().get();

lib/icinga/checkable-flapping.cpp

@@ -109,6 +109,6 @@ int Checkable::ServiceStateToFlappingFilter(ServiceState state)
 		case ServiceUnknown:
 			return StateFilterUnknown;
 		default:
-			VERIFY(!"Invalid state type.");
+			ABORT("Invalid state type.");
 	}
 }

lib/icinga/notification.cpp

@@ -648,7 +648,7 @@ int icinga::ServiceStateToFilter(ServiceState state)
 		case ServiceUnknown:
 			return StateFilterUnknown;
 		default:
-			VERIFY(!"Invalid state type.");
+			ABORT("Invalid state type.");
 	}
 }
 
@@ -660,7 +660,7 @@ int icinga::HostStateToFilter(HostState state)
 		case HostDown:
 			return StateFilterDown;
 		default:
-			VERIFY(!"Invalid state type.");
+			ABORT("Invalid state type.");
 	}
 }
 
@@ -736,7 +736,7 @@ String Notification::NotificationServiceStateToString(ServiceState state)
 		case ServiceUnknown:
 			return "Unknown";
 		default:
-			VERIFY(!"Invalid state type.");
+			ABORT("Invalid state type.");
 	}
 }
 
@@ -748,7 +748,7 @@ String Notification::NotificationHostStateToString(HostState state)
 		case HostDown:
 			return "Down";
 		default:
-			VERIFY(!"Invalid state type.");
+			ABORT("Invalid state type.");
 	}
 }
 

lib/icingadb/icingadb-objects.cpp

@@ -549,7 +549,7 @@ std::vector<std::vector<intrusive_ptr<ConfigObject>>> IcingaDB::ChunkObjects(std
 
 	chunks.reserve((std::distance(offset, end) + chunkSize - 1) / chunkSize);
 
-	while (std::distance(offset, end) >= chunkSize) {
+	while (static_cast<std::size_t>(std::distance(offset, end)) >= chunkSize) {
 		auto until (offset + chunkSize);
 		chunks.emplace_back(offset, until);
 		offset = until;
@@ -1375,7 +1375,7 @@ void IcingaDB::UpdateDependenciesState(const Checkable::Ptr& checkable, const De
 	}
 
 	RedisConnection::Queries streamStates;
-	auto addDependencyStateToStream([this, &streamStates](const String& redisKey, const Dictionary::Ptr& stateAttrs) {
+	auto addDependencyStateToStream([&streamStates](const String& redisKey, const Dictionary::Ptr& stateAttrs) {
 		RedisConnection::Query xAdd{
 			"XADD", "icinga:runtime:state", "MAXLEN", "~", "1000000", "*", "runtime_type", "upsert",
 			"redis_key", redisKey

lib/perfdata/graphitewriter.cpp

@@ -288,7 +288,7 @@ void GraphiteWriter::CheckResultHandler(const Checkable::Ptr& checkable, const C
 	std::vector<std::pair<String, double>> metadata;
 	if (GetEnableSendMetadata()) {
 		metadata = {
-			{"state", service ? service->GetState() : host->GetState()},
+			{"state", service ? static_cast<unsigned int>(service->GetState()) : static_cast<unsigned int>(host->GetState())},
 			{"current_attempt", checkable->GetCheckAttempt()},
 			{"max_check_attempts", checkable->GetMaxCheckAttempts()},
 			{"state_type", checkable->GetStateType()},

lib/remote/apilistener-authority.cpp

@@ -25,7 +25,7 @@ void ApiListener::UpdateObjectAuthority()
 
 	std::vector<Endpoint::Ptr> endpoints;
 	Endpoint::Ptr my_endpoint;
-	int hostChildrenInheritObjectAuthority = 0;
+	std::size_t hostChildrenInheritObjectAuthority = 0;
 
 	if (my_zone) {
 		my_endpoint = Endpoint::GetLocalEndpoint();

lib/remote/apilistener-filesync.cpp

@@ -104,9 +104,6 @@ void ApiListener::SyncLocalZoneDir(const Zone::Ptr& zone) const
 
 	Utility::MkDirP(productionZonesDir, 0700);
 
-	// Copy content and add additional meta data.
-	size_t numBytes = 0;
-
 	/* Note: We cannot simply copy directories here.
 	 *
 	 * Zone directories are registered from everywhere and we already
@@ -131,8 +128,6 @@ void ApiListener::SyncLocalZoneDir(const Zone::Ptr& zone) const
 
 			fp << content;
 			fp.close();
-
-			numBytes += content.GetLength();
 		}
 	}
 

lib/remote/apilistener.cpp

@@ -1004,11 +1004,11 @@ void ApiListener::ApiTimerHandler()
 {
 	double now = Utility::GetTime();
 
-	std::vector<int> files;
+	std::vector<std::uint64_t> files;
 	Utility::Glob(GetApiDir() + "log/*", [&files](const String& file) { LogGlobHandler(files, file); }, GlobFile);
 	std::sort(files.begin(), files.end());
 
-	for (int ts : files) {
+	for (auto ts : files) {
 		bool need = false;
 		auto localZone (GetLocalEndpoint()->GetZone());
 
@@ -1468,7 +1468,7 @@ void ApiListener::RotateLogFile()
 		ts = Utility::GetTime();
 
 	String oldpath = GetApiDir() + "log/current";
-	String newpath = GetApiDir() + "log/" + Convert::ToString(static_cast<int>(ts)+1);
+	String newpath = GetApiDir() + "log/" + Convert::ToString(static_cast<std::uint64_t>(ts)+1);
 
 	// If the log is being rotated more than once per second,
 	// don't overwrite the previous one, but silently deny rotation.
@@ -1486,22 +1486,20 @@ void ApiListener::RotateLogFile()
 	}
 }
 
-void ApiListener::LogGlobHandler(std::vector<int>& files, const String& file)
+void ApiListener::LogGlobHandler(std::vector<std::uint64_t>& files, const String& file)
 {
 	String name = Utility::BaseName(file);
 
 	if (name == "current")
 		return;
 
-	int ts;
-
 	try {
-		ts = Convert::ToLong(name);
+		files.emplace_back(boost::lexical_cast<std::uint64_t>(name));
-	} catch (const std::exception&) {
+	} catch (const std::exception& ex) {
+		Log(LogCritical, "ApiListener")
+			<< "Error converting log file name " << file << " to uint64: " << ex.what();
 		return;
 	}
-
-	files.push_back(ts);
 }
 
 void ApiListener::ReplayLog(const JsonRpcConnection::Ptr& client)
@@ -1548,19 +1546,19 @@ void ApiListener::ReplayLog(const JsonRpcConnection::Ptr& client)
 
 		count = 0;
 
-		std::vector<int> files;
+		std::vector<std::uint64_t> files;
 		Utility::Glob(GetApiDir() + "log/*", [&files](const String& file) { LogGlobHandler(files, file); }, GlobFile);
 		std::sort(files.begin(), files.end());
 
-		std::vector<std::pair<int, String>> allFiles;
+		std::vector<std::pair<std::uint64_t, String>> allFiles;
 
-		for (int ts : files) {
+		for (auto ts : files) {
 			if (ts >= peer_ts) {
 				allFiles.emplace_back(ts, GetApiDir() + "log/" + Convert::ToString(ts));
 			}
 		}
 
-		allFiles.emplace_back(Utility::GetTime() + 1, GetApiDir() + "log/current");
+		allFiles.emplace_back(static_cast<std::uint64_t>(Utility::GetTime()) + 1, GetApiDir() + "log/current");
 
 		for (auto& file : allFiles) {
 			Log(LogNotice, "ApiListener")

lib/remote/apilistener.hpp

@@ -230,7 +230,7 @@ private:
 	void OpenLogFile();
 	void RotateLogFile();
 	void CloseLogFile();
-	static void LogGlobHandler(std::vector<int>& files, const String& file);
+	static void LogGlobHandler(std::vector<std::uint64_t>& files, const String& file);
 	void ReplayLog(const JsonRpcConnection::Ptr& client);
 
 	static void CopyCertificateFile(const String& oldCertPath, const String& newCertPath);

lib/remote/consolehandler.cpp

@@ -21,7 +21,6 @@ using namespace icinga;
 
 REGISTER_URLHANDLER("/v1/console", ConsoleHandler);
 
-static std::mutex l_QueryMutex;
 static std::map<String, ApiScriptFrame> l_ApiScriptFrames;
 static Timer::Ptr l_FrameCleanupTimer;
 static std::mutex l_ApiScriptMutex;

test/base-stacktrace.cpp

@@ -20,13 +20,14 @@ using namespace icinga;
  *  should be printed. If it looks somewhat meaningful, you can probably ignore a failure of this test case.
  */
 
-#ifndef _MSC_VER
+#if defined(__GNUC__) && !defined(__clang__)
 #pragma GCC push_options
-#pragma GCC optimize ("O0")
+#pragma GCC optimize("O0")
+#elif defined(__clang__)
 #pragma clang optimize off
-#else /* _MSC_VER */
+#elif defined(_MSC_VER)
 #pragma optimize("", off)
-#endif /* _MSC_VER */
+#endif
 
 BOOST_AUTO_TEST_SUITE(base_stacktrace)
 
@@ -66,9 +67,10 @@ BOOST_AUTO_TEST_CASE(stacktrace)
 
 BOOST_AUTO_TEST_SUITE_END()
 
-#ifndef _MSC_VER
+#if defined(__GNUC__) && !defined(__clang__)
 #pragma GCC pop_options
+#elif defined(__clang__)
 #pragma clang optimize on
-#else /* _MSC_VER */
+#elif defined(_MSC_VER)
 #pragma optimize("", on)
-#endif /* _MSC_VER */
+#endif

test/icinga-dependencies.cpp

@@ -35,7 +35,12 @@ static void RegisterDependency(Dependency::Ptr dep, const String& redundancyGrou
 	dep->GetParent()->AddReverseDependency(dep);
 }
 
-static void AssertCheckableRedundancyGroup(Checkable::Ptr checkable, int dependencyCount, int groupCount, int totalDependenciesCount)
+static void AssertCheckableRedundancyGroup(
+	Checkable::Ptr checkable,
+	std::size_t dependencyCount,
+	std::size_t groupCount,
+	std::size_t totalDependenciesCount
+)
 {
 	BOOST_CHECK_MESSAGE(
 		dependencyCount == checkable->GetDependencies().size(),

test/icinga-legacytimeperiod.cpp

@@ -397,10 +397,6 @@ void AdvancedHelper(const char *timestamp, DateTime from, DateTime to)
 BOOST_AUTO_TEST_CASE(advanced)
 {
 	String timestamp;
-	boost::posix_time::ptime begin;
-	boost::posix_time::ptime end;
-	boost::posix_time::ptime expectedBegin;
-	boost::posix_time::ptime expectedEnd;
 
 	//-----------------------------------------------------
 	// 2019-05-06 where Icinga celebrates 10 years #monitoringlove

test/remote-httpserverconnection.cpp

@@ -56,7 +56,7 @@ struct HttpServerConnectionFixture : TlsStreamFixture, ConfigurationCacheDirFixt
 	template<class Rep, class Period>
 	bool AssertServerDisconnected(const std::chrono::duration<Rep, Period>& timeout)
 	{
-		auto iterations = timeout / std::chrono::milliseconds(50);
+		std::size_t iterations = timeout / std::chrono::milliseconds(50);
 		for (std::size_t i = 0; i < iterations && !m_Connection->Disconnected(); i++) {
 			Utility::Sleep(std::chrono::duration<double>(timeout).count() / iterations);
 		}

tools/mkclass/classcompiler.cpp

@@ -1477,26 +1477,32 @@ void ClassCompiler::CompileStream(const std::string& path, std::istream& input,
 		<< "#include \"base/logger.hpp\"" << std::endl
 		<< "#include \"base/function.hpp\"" << std::endl
 		<< "#include \"base/configobject.hpp\"" << std::endl
-		<< "#include \"base/configtype.hpp\"" << std::endl
+		<< "#include \"base/configtype.hpp\"" << std::endl;
-		<< "#ifdef _MSC_VER" << std::endl
-		<< "#pragma warning( push )" << std::endl
-		<< "#pragma warning( disable : 4244 )" << std::endl
-		<< "#pragma warning( disable : 4800 )" << std::endl
-		<< "#else /* _MSC_VER */" << std::endl
-		<< "#pragma GCC diagnostic push" << std::endl
-		<< "#pragma GCC diagnostic ignored \"-Wunused-parameter\"" << std::endl
-		<< "#pragma GCC diagnostic ignored \"-Wunused-variable\"" << std::endl
-		<< "#endif /* _MSC_VER */" << std::endl << std::endl;
 
+#ifdef _MSC_VER
+	oimpl << "#pragma warning( push )" << std::endl
+		<< "#pragma warning( disable : 4244 )" << std::endl
+		<< "#pragma warning( disable : 4800 )" << std::endl;
+#elif defined(__GNUC__) && !defined(__clang__)
+	oimpl << "#pragma GCC diagnostic push" << std::endl
+		<< "#pragma GCC diagnostic ignored \"-Wunused-parameter\"" << std::endl
+		<< "#pragma GCC diagnostic ignored \"-Wunused-variable\"" << std::endl;
+#elif defined(__clang__)
+	oimpl << "#pragma clang diagnostic push" << std::endl
+		<< "#pragma clang diagnostic ignored \"-Wunused-parameter\"" << std::endl
+		<< "#pragma clang diagnostic ignored \"-Wunused-variable\"" << std::endl;
+#endif /* _MSC_VER */
 
 	ClassCompiler ctx(path, input, oimpl, oheader);
 	ctx.Compile();
 
 	oheader << "#endif /* " << guard_name << " */" << std::endl;
 
-	oimpl << "#ifdef _MSC_VER" << std::endl
+#ifdef _MSC_VER
-		<< "#pragma warning ( pop )" << std::endl
+		oimpl << "#pragma warning ( pop )" << std::endl;
-		<< "#else /* _MSC_VER */" << std::endl
+#elif defined(__GNUC__) && !defined(__clang__)
-		<< "#pragma GCC diagnostic pop" << std::endl
+		oimpl << "#pragma GCC diagnostic pop" << std::endl;
-		<< "#endif /* _MSC_VER */" << std::endl;
+#elif defined(__clang__)
+		oimpl << "#pragma clang diagnostic pop" << std::endl;
+#endif /* _MSC_VER */
 }