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

yhabteab · web-flow · commit ed9014103fd0 · 2025-11-19T14:26:57.000+01:00
Fix misc compiler warnings
diff --git a/lib/base/application.hpp b/lib/base/application.hpp
@@ -119,7 +119,7 @@ class Application : public ObjectImpl<Application> {
 
 	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. */
diff --git a/lib/base/debug.hpp b/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 NORETURNPRE __declspec(noreturn)
-#else /* _MSC_VER */
-#	define NORETURNPRE
-#endif /* _MSC_VER */
+#define ABORT(expr) icinga_assert_fail(#expr, __FILE__, __LINE__)
 
-#ifdef __GNUC__
-#	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)
+[[noreturn]] inline void icinga_assert_fail(const char *expr, const char *file, int line) noexcept(true)
 {
 	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 */
diff --git a/lib/base/exception.hpp b/lib/base/exception.hpp
@@ -35,7 +35,7 @@ class ScriptError : virtual public user_error
 	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 @@ class ValidationError : virtual public user_error
 	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 @@ class invalid_downtime_removal_error : virtual public std::exception, virtual pu
 
 	~invalid_downtime_removal_error() noexcept override;
 
-	const char *what() const noexcept final;
+	const char* what() const noexcept override final;
 
 private:
 	String m_Message;
diff --git a/lib/base/io-engine.cpp b/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();
 	}
 }
diff --git a/lib/base/logger.hpp b/lib/base/logger.hpp
@@ -88,7 +88,7 @@ class Logger : public ObjectImpl<Logger>
 	}
 
 	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;
diff --git a/lib/base/netstring.cpp b/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";
 
diff --git a/lib/base/object-packer.cpp b/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;
-	PackAny(value, builder);
+	String builder;
+	PackAny(value, builder.GetData());
 
 	return builder;
 }
diff --git a/lib/cli/clicommand.cpp b/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;
 			}
 
diff --git a/lib/config/configitem.cpp b/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() << "'.";
diff --git a/lib/config/expression.cpp b/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
diff --git a/lib/icinga/checkable-dependency.cpp b/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();
diff --git a/lib/icinga/checkable-flapping.cpp b/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.");
 	}
 }
diff --git a/lib/icinga/notification.cpp b/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.");
 	}
 }
 
diff --git a/lib/icingadb/icingadb-objects.cpp b/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
diff --git a/lib/perfdata/graphitewriter.cpp b/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()},
diff --git a/lib/remote/apilistener-authority.cpp b/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();
diff --git a/lib/remote/apilistener-filesync.cpp b/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();
 		}
 	}
 
diff --git a/lib/remote/apilistener.cpp b/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);
-	} catch (const std::exception&) {
+		files.emplace_back(boost::lexical_cast<std::uint64_t>(name));
+	} 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")
diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp
@@ -230,7 +230,7 @@ class ApiListener final : public ObjectImpl<ApiListener>
 	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);
diff --git a/lib/remote/consolehandler.cpp b/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;
diff --git a/test/base-stacktrace.cpp b/test/base-stacktrace.cpp
diff --git a/test/icinga-dependencies.cpp b/test/icinga-dependencies.cpp
diff --git a/test/icinga-legacytimeperiod.cpp b/test/icinga-legacytimeperiod.cpp
diff --git a/test/remote-httpserverconnection.cpp b/test/remote-httpserverconnection.cpp
diff --git a/tools/mkclass/classcompiler.cpp b/tools/mkclass/classcompiler.cpp

Original file line number	Diff line number	Diff line change
`@@ -101,9 +101,7 @@ IoEngine::~IoEngine()`
`101`	`101`	`boost::asio::post(m_IoContext, []() {`
`102`	`102`	`throw TerminateIoThread();`
`103`	`103`	`});`
`104`		`- }`
`105`	`104`
`106`		`- for (auto& thread : m_Threads) {`
`107`	`105`	`thread.join();`
`108`	`106`	`}`
`109`	`107`	`}`
Original file line number	Diff line number	Diff line change
`@@ -88,7 +88,7 @@ class Logger : public ObjectImpl<Logger>`
`88`	`88`	`}`
`89`	`89`
`90`	`90`	`void SetSeverity(const String& value, bool suppress_events = false, const Value& cookie = Empty) override;`
`91`		`- void ValidateSeverity(const Lazy<String>& lvalue, const ValidationUtils& utils) final;`
	`91`	`+ void ValidateSeverity(const Lazy<String>& lvalue, const ValidationUtils& utils) override final;`
`92`	`92`
`93`	`93`	`protected:`
`94`	`94`	`void Start(bool runtimeCreated) override;`
Original file line number	Diff line number	Diff line change
`@@ -167,7 +167,7 @@ String NetString::ReadStringFromStream(const Shared<AsioTlsStream>::Ptr& stream,`
`167`	`167`	`}`
`168`	`168`	`}`
`169`	`169`
`170`		`- if (maxMessageLength >= 0 && len > maxMessageLength) {`
	`170`	`+ if (maxMessageLength >= 0 && len > static_cast<std::size_t>(maxMessageLength)) {`
`171`	`171`	`std::stringstream errorMessage;`
`172`	`172`	`errorMessage << "Max data length exceeded: " << (maxMessageLength / 1024) << " KB";`
`173`	`173`
`@@ -246,7 +246,7 @@ String NetString::ReadStringFromStream(const Shared<AsioTlsStream>::Ptr& stream,`
`246`	`246`	`}`
`247`	`247`	`}`
`248`	`248`
`249`		`- if (maxMessageLength >= 0 && len > maxMessageLength) {`
	`249`	`+ if (maxMessageLength >= 0 && len > static_cast<std::size_t>(maxMessageLength)) {`
`250`	`250`	`std::stringstream errorMessage;`
`251`	`251`	`errorMessage << "Max data length exceeded: " << (maxMessageLength / 1024) << " KB";`
`252`	`252`
Original file line number	Diff line number	Diff line change
`@@ -239,8 +239,8 @@ static void PackAny(const Value& value, std::string& builder)`
`239`	`239`	`*/`
`240`	`240`	`String icinga::PackObject(const Value& value)`
`241`	`241`	`{`
`242`		`- std::string builder;`
`243`		`- PackAny(value, builder);`
	`242`	`+ String builder;`
	`243`	`+ PackAny(value, builder.GetData());`
`244`	`244`
`245`	`245`	`return builder;`
`246`	`246`	`}`
Original file line number	Diff line number	Diff line change
`@@ -554,7 +554,7 @@ ExpressionResult GetScopeExpression::DoEvaluate(ScriptFrame& frame, DebugHint *d`
`554`	`554`	`else if (m_ScopeSpec == ScopeGlobal)`
`555`	`555`	`return frame.GetGlobals();`
`556`	`556`	`else`
`557`		`- VERIFY(!"Invalid scope.");`
	`557`	`+ ABORT("Invalid scope.");`
`558`	`558`	`}`
`559`	`559`
`560`	`560`	`static inline`