From 5598f4d0d6599cc5c98af524df5ed52143b7e38b Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Wed, 30 Oct 2024 13:46:07 -0400 Subject: [PATCH 1/4] Simplify OutputStream --- cpp/include/Ice/OutputStream.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/cpp/include/Ice/OutputStream.h b/cpp/include/Ice/OutputStream.h index 420b04dfc17..4441bb8af6f 100644 --- a/cpp/include/Ice/OutputStream.h +++ b/cpp/include/Ice/OutputStream.h @@ -601,14 +601,6 @@ namespace Ice */ void write(const double* begin, const double* end); - /** - * Writes a string to the stream. - * @param v The string to write. - * @param convert Determines whether the string is processed by the narrow string converter, - * if one has been configured. The default behavior is to convert the strings. - */ - void write(const std::string& v, bool convert = true) { write(std::string_view(v), convert); } - /** * Writes a string view to the stream. * @param v The string view to write. From 826afbec35e84edc9cdc663054ccfc68550aba0c Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Wed, 30 Oct 2024 14:40:29 -0400 Subject: [PATCH 2/4] Update Logger --- cpp/include/Ice/Logger.h | 6 +++++- cpp/include/Ice/OutputStream.h | 8 ++++++++ cpp/src/Ice/LoggerAdminI.cpp | 7 ++++--- cpp/src/Ice/LoggerI.cpp | 12 ++++++------ cpp/src/Ice/LoggerI.h | 16 ++++++++-------- cpp/src/Ice/OSLogLoggerI.cpp | 18 +++++++++--------- cpp/src/Ice/OSLogLoggerI.h | 18 +++++++++--------- cpp/src/Ice/Service.cpp | 18 +++++++++--------- cpp/src/Ice/SysLoggerI.cpp | 8 ++++---- cpp/src/Ice/SysLoggerI.h | 18 +++++++++--------- cpp/src/Ice/SystemdJournalI.cpp | 6 +++--- cpp/src/Ice/SystemdJournalI.h | 18 +++++++++--------- cpp/test/Ice/admin/TestI.cpp | 2 +- cpp/test/Ice/binding/Server.cpp | 19 +++++++------------ python/modules/IcePy/Logger.cpp | 2 +- python/modules/IcePy/Logger.h | 14 +++++++------- swift/src/IceImpl/LoggerWrapperI.h | 16 +++++++--------- 17 files changed, 106 insertions(+), 100 deletions(-) diff --git a/cpp/include/Ice/Logger.h b/cpp/include/Ice/Logger.h index 4f3477e803c..90e1b0338ed 100644 --- a/cpp/include/Ice/Logger.h +++ b/cpp/include/Ice/Logger.h @@ -22,6 +22,10 @@ namespace Ice public: virtual ~Logger() = default; + // We use const std::string& and not std::string_view for the log messages because implementations commonly + // send the message to C APIs that require null-terminated strings. + // The message itself is also often constructed from a string produced by an ostringstream. + /** * Print a message. The message is printed literally, without any decorations such as executable name or time * stamp. @@ -61,7 +65,7 @@ namespace Ice * @param prefix The new prefix for the logger. * @return A logger instance. */ - virtual LoggerPtr cloneWithPrefix(const std::string& prefix) = 0; + virtual LoggerPtr cloneWithPrefix(std::string prefix) = 0; }; } diff --git a/cpp/include/Ice/OutputStream.h b/cpp/include/Ice/OutputStream.h index 4441bb8af6f..420b04dfc17 100644 --- a/cpp/include/Ice/OutputStream.h +++ b/cpp/include/Ice/OutputStream.h @@ -601,6 +601,14 @@ namespace Ice */ void write(const double* begin, const double* end); + /** + * Writes a string to the stream. + * @param v The string to write. + * @param convert Determines whether the string is processed by the narrow string converter, + * if one has been configured. The default behavior is to convert the strings. + */ + void write(const std::string& v, bool convert = true) { write(std::string_view(v), convert); } + /** * Writes a string view to the stream. * @param v The string view to write. diff --git a/cpp/src/Ice/LoggerAdminI.cpp b/cpp/src/Ice/LoggerAdminI.cpp index 787ee06073d..d8b02ba9434 100644 --- a/cpp/src/Ice/LoggerAdminI.cpp +++ b/cpp/src/Ice/LoggerAdminI.cpp @@ -111,7 +111,8 @@ namespace void warning(const std::string&) final; void error(const std::string&) final; std::string getPrefix() final; - LoggerPtr cloneWithPrefix(const std::string&) final; + LoggerPtr cloneWithPrefix(std::string) final; + ObjectPtr getFacet() const; void destroy() final; @@ -629,9 +630,9 @@ namespace string LoggerAdminLoggerI::getPrefix() { return _localLogger->getPrefix(); } - LoggerPtr LoggerAdminLoggerI::cloneWithPrefix(const string& prefix) + LoggerPtr LoggerAdminLoggerI::cloneWithPrefix(string prefix) { - return _localLogger->cloneWithPrefix(prefix); + return _localLogger->cloneWithPrefix(std::move(prefix)); } ObjectPtr LoggerAdminLoggerI::getFacet() const { return _loggerAdmin; } diff --git a/cpp/src/Ice/LoggerI.cpp b/cpp/src/Ice/LoggerI.cpp index d6163efb645..e7c8f22721e 100644 --- a/cpp/src/Ice/LoggerI.cpp +++ b/cpp/src/Ice/LoggerI.cpp @@ -25,15 +25,15 @@ namespace const chrono::minutes retryTimeout = chrono::minutes(5); } -Ice::LoggerI::LoggerI(const string& prefix, const string& file, bool convert, size_t sizeMax) - : _prefix(prefix), +Ice::LoggerI::LoggerI(string prefix, const string& file, bool convert, size_t sizeMax) + : _prefix(std::move(prefix)), _convert(convert), _converter(getProcessStringConverter()), _sizeMax(sizeMax) { - if (!prefix.empty()) + if (!_prefix.empty()) { - _formattedPrefix = prefix + ": "; + _formattedPrefix = _prefix + ": "; } if (!file.empty()) @@ -102,10 +102,10 @@ Ice::LoggerI::getPrefix() } LoggerPtr -Ice::LoggerI::cloneWithPrefix(const std::string& prefix) +Ice::LoggerI::cloneWithPrefix(std::string prefix) { lock_guard lock(outputMutex); // for _sizeMax - return make_shared(prefix, _file, _convert, _sizeMax); + return make_shared(std::move(prefix), _file, _convert, _sizeMax); } void diff --git a/cpp/src/Ice/LoggerI.h b/cpp/src/Ice/LoggerI.h index e498ea0b04c..3d2475076cc 100644 --- a/cpp/src/Ice/LoggerI.h +++ b/cpp/src/Ice/LoggerI.h @@ -12,18 +12,18 @@ namespace Ice { - class LoggerI : public Logger + class LoggerI final : public Logger { public: - LoggerI(const std::string&, const std::string&, bool convert = true, std::size_t sizeMax = 0); + LoggerI(std::string prefix, const std::string&, bool convert = true, std::size_t sizeMax = 0); ~LoggerI(); - virtual void print(const std::string&); - virtual void trace(const std::string&, const std::string&); - virtual void warning(const std::string&); - virtual void error(const std::string&); - virtual std::string getPrefix(); - virtual LoggerPtr cloneWithPrefix(const std::string&); + void print(const std::string&) final; + void trace(const std::string& category, const std::string& message) final; + void warning(const std::string&) final; + void error(const std::string&) final; + std::string getPrefix() final; + LoggerPtr cloneWithPrefix(std::string) final; private: void write(const std::string&, bool); diff --git a/cpp/src/Ice/OSLogLoggerI.cpp b/cpp/src/Ice/OSLogLoggerI.cpp index 74068f1b37d..f7ffbdcc35d 100644 --- a/cpp/src/Ice/OSLogLoggerI.cpp +++ b/cpp/src/Ice/OSLogLoggerI.cpp @@ -10,20 +10,20 @@ using namespace std; using namespace Ice; -Ice::OSLogLoggerI::OSLogLoggerI(const std::string& prefix) : _prefix(prefix) +Ice::OSLogLoggerI::OSLogLoggerI(string prefix) : _prefix(std::move(prefix)) { - const string subsystem = prefix.empty() ? "com.zeroc.ice" : "com.zeroc.ice." + prefix; + const string subsystem = _prefix.empty() ? "com.zeroc.ice" : "com.zeroc.ice." + _prefix; _log.reset(os_log_create(subsystem.c_str(), "")); } void -Ice::OSLogLoggerI::print(const std::string& message) +Ice::OSLogLoggerI::print(const string& message) { os_log_with_type(_log.get(), OS_LOG_TYPE_DEFAULT, "%{public}s.", message.c_str()); } void -Ice::OSLogLoggerI::trace(const std::string& category, const std::string& message) +Ice::OSLogLoggerI::trace(const string& category, const string& message) { const string subsystem = _prefix.empty() ? "com.zeroc.ice" : "com.zeroc.ice." + _prefix; IceInternal::UniqueRef log(os_log_create(subsystem.c_str(), category.c_str())); @@ -31,27 +31,27 @@ Ice::OSLogLoggerI::trace(const std::string& category, const std::string& message } void -Ice::OSLogLoggerI::warning(const std::string& message) +Ice::OSLogLoggerI::warning(const string& message) { os_log_with_type(_log.get(), OS_LOG_TYPE_ERROR, "%{public}s.", message.c_str()); } void -Ice::OSLogLoggerI::error(const std::string& message) +Ice::OSLogLoggerI::error(const string& message) { os_log_with_type(_log.get(), OS_LOG_TYPE_FAULT, "%{public}s.", message.c_str()); } -std::string +string Ice::OSLogLoggerI::getPrefix() { return _prefix; } LoggerPtr -Ice::OSLogLoggerI::cloneWithPrefix(const std::string& prefix) +Ice::OSLogLoggerI::cloneWithPrefix(string prefix) { - return make_shared(prefix); + return make_shared(std::move(prefix)); } #endif diff --git a/cpp/src/Ice/OSLogLoggerI.h b/cpp/src/Ice/OSLogLoggerI.h index 3b3e9aa609b..ac5ccadb2d2 100644 --- a/cpp/src/Ice/OSLogLoggerI.h +++ b/cpp/src/Ice/OSLogLoggerI.h @@ -13,17 +13,17 @@ namespace Ice { - class OSLogLoggerI : public Logger + class OSLogLoggerI final : public Logger { public: - OSLogLoggerI(const std::string&); - - virtual void print(const std::string&); - virtual void trace(const std::string&, const std::string&); - virtual void warning(const std::string&); - virtual void error(const std::string&); - virtual std::string getPrefix(); - virtual LoggerPtr cloneWithPrefix(const std::string&); + OSLogLoggerI(std::string); + + void print(const std::string&) final; + void trace(const std::string&, const std::string&) final; + void warning(const std::string&) final; + void error(const std::string&) final; + std::string getPrefix() final; + LoggerPtr cloneWithPrefix(std::string) final; private: const std::string _prefix; diff --git a/cpp/src/Ice/Service.cpp b/cpp/src/Ice/Service.cpp index 253d8916413..6fef219963b 100644 --- a/cpp/src/Ice/Service.cpp +++ b/cpp/src/Ice/Service.cpp @@ -117,27 +117,27 @@ namespace }; using SMEventLoggerPtr = std::shared_ptr; - class SMEventLoggerIWrapper : public Ice::Logger + class SMEventLoggerIWrapper final : public Ice::Logger { public: - SMEventLoggerIWrapper(const SMEventLoggerPtr& logger, const string& prefix) : _logger(logger), _prefix(prefix) + SMEventLoggerIWrapper(const SMEventLoggerPtr& logger, string prefix) : _logger(logger), _prefix(std::move(prefix)) { assert(_logger); } - virtual void print(const string& message) { _logger->print(_prefix, message); } + void print(const string& message) final { _logger->print(_prefix, message); } - void trace(const string& category, const string& message) { _logger->trace(_prefix, category, message); } + void trace(const string& category, const string& message) final { _logger->trace(_prefix, category, message); } - virtual void warning(const string& message) { _logger->warning(_prefix, message); } + void warning(const string& message) final { _logger->warning(_prefix, message); } - virtual void error(const string& message) { _logger->error(_prefix, message); } + void error(const string& message) final { _logger->error(_prefix, message); } - virtual string getPrefix() { return _prefix; } + string getPrefix() final { return _prefix; } - virtual Ice::LoggerPtr cloneWithPrefix(const string& prefix) + Ice::LoggerPtr cloneWithPrefix(string prefix) final { - return make_shared(_logger, prefix); + return make_shared(_logger, std::move(prefix)); } private: diff --git a/cpp/src/Ice/SysLoggerI.cpp b/cpp/src/Ice/SysLoggerI.cpp index f68a7e45db1..516284344c1 100644 --- a/cpp/src/Ice/SysLoggerI.cpp +++ b/cpp/src/Ice/SysLoggerI.cpp @@ -12,7 +12,7 @@ using namespace std; using namespace Ice; using namespace IceInternal; -Ice::SysLoggerI::SysLoggerI(const string& prefix, const string& facilityString) : _facility(0), _prefix(prefix) +Ice::SysLoggerI::SysLoggerI(string prefix, const string& facilityString) : _facility(0), _prefix(std::move(prefix)) { if (facilityString == "LOG_KERN") { @@ -107,7 +107,7 @@ Ice::SysLoggerI::SysLoggerI(const string& prefix, const string& facilityString) openlog(_prefix.c_str(), logopt, _facility); } -Ice::SysLoggerI::SysLoggerI(const string& prefix, int facility) : _facility(facility), _prefix(prefix) +Ice::SysLoggerI::SysLoggerI(string prefix, int facility) : _facility(facility), _prefix(std::move(prefix)) { int logopt = LOG_PID | LOG_CONS; openlog(_prefix.c_str(), logopt, facility); @@ -151,9 +151,9 @@ Ice::SysLoggerI::getPrefix() } Ice::LoggerPtr -Ice::SysLoggerI::cloneWithPrefix(const string& prefix) +Ice::SysLoggerI::cloneWithPrefix(string prefix) { - return make_shared(prefix, _facility); + return make_shared(std::move(prefix), _facility); } #endif diff --git a/cpp/src/Ice/SysLoggerI.h b/cpp/src/Ice/SysLoggerI.h index fdd866420f7..e373cd1c297 100644 --- a/cpp/src/Ice/SysLoggerI.h +++ b/cpp/src/Ice/SysLoggerI.h @@ -11,19 +11,19 @@ namespace Ice { - class SysLoggerI : public Logger + class SysLoggerI final : public Logger { public: - SysLoggerI(const std::string&, const std::string&); - SysLoggerI(const std::string&, int); + SysLoggerI(std::string prefix, const std::string& facilityString); + SysLoggerI(std::string prefix, int facility); ~SysLoggerI(); - virtual void print(const std::string&); - virtual void trace(const std::string&, const std::string&); - virtual void warning(const std::string&); - virtual void error(const std::string&); - virtual std::string getPrefix(); - virtual LoggerPtr cloneWithPrefix(const std::string&); + void print(const std::string&) final; + void trace(const std::string&, const std::string&) final; + void warning(const std::string&) final; + void error(const std::string&) final; + std::string getPrefix() final; + LoggerPtr cloneWithPrefix(std::string) final; private: int _facility; diff --git a/cpp/src/Ice/SystemdJournalI.cpp b/cpp/src/Ice/SystemdJournalI.cpp index 96158479b33..43fd1a037f9 100644 --- a/cpp/src/Ice/SystemdJournalI.cpp +++ b/cpp/src/Ice/SystemdJournalI.cpp @@ -13,7 +13,7 @@ using namespace std; using namespace Ice; using namespace IceInternal; -Ice::SystemdJournalI::SystemdJournalI(const string& prefix) : _prefix(prefix) {} +Ice::SystemdJournalI::SystemdJournalI(string prefix) : _prefix(std::move(prefix)) {} void Ice::SystemdJournalI::print(const string& message) @@ -46,9 +46,9 @@ Ice::SystemdJournalI::getPrefix() } Ice::LoggerPtr -Ice::SystemdJournalI::cloneWithPrefix(const string& prefix) +Ice::SystemdJournalI::cloneWithPrefix(string prefix) { - return make_shared(prefix); + return make_shared(std::move(prefix)); } void diff --git a/cpp/src/Ice/SystemdJournalI.h b/cpp/src/Ice/SystemdJournalI.h index cdad3eb0b98..1e86d632f24 100644 --- a/cpp/src/Ice/SystemdJournalI.h +++ b/cpp/src/Ice/SystemdJournalI.h @@ -11,17 +11,17 @@ namespace Ice { - class SystemdJournalI : public Logger + class SystemdJournalI final : public Logger { public: - SystemdJournalI(const std::string&); - - virtual void print(const std::string&); - virtual void trace(const std::string&, const std::string&); - virtual void warning(const std::string&); - virtual void error(const std::string&); - virtual std::string getPrefix(); - virtual LoggerPtr cloneWithPrefix(const std::string&); + SystemdJournalI(std::string); + + void print(const std::string&) final; + void trace(const std::string&, const std::string&) final; + void warning(const std::string&) final; + void error(const std::string&) final; + std::string getPrefix() final; + LoggerPtr cloneWithPrefix(std::string) final; private: void write(int, const std::string&) const; diff --git a/cpp/test/Ice/admin/TestI.cpp b/cpp/test/Ice/admin/TestI.cpp index 2044267df3c..a7486c718f7 100644 --- a/cpp/test/Ice/admin/TestI.cpp +++ b/cpp/test/Ice/admin/TestI.cpp @@ -28,7 +28,7 @@ namespace string getPrefix() final { return "NullLogger"; } - LoggerPtr cloneWithPrefix(const string&) final { return shared_from_this(); } + LoggerPtr cloneWithPrefix(string) final { return shared_from_this(); } }; } diff --git a/cpp/test/Ice/binding/Server.cpp b/cpp/test/Ice/binding/Server.cpp index da93372a512..ca8df603d1a 100644 --- a/cpp/test/Ice/binding/Server.cpp +++ b/cpp/test/Ice/binding/Server.cpp @@ -14,20 +14,15 @@ namespace // A no-op Logger, used when testing the Logger Admin // - class NullLogger : public Ice::Logger, public std::enable_shared_from_this + class NullLogger final : public Ice::Logger, public std::enable_shared_from_this { public: - virtual void print(const string&) {} - - virtual void trace(const string&, const string&) {} - - virtual void warning(const string&) {} - - virtual void error(const string&) {} - - virtual string getPrefix() { return "NullLogger"; } - - virtual Ice::LoggerPtr cloneWithPrefix(const string&) { return shared_from_this(); } + void print(const string&) final {} + void trace(const string&, const string&) final {} + void warning(const string&) final {} + void error(const string&) final {} + string getPrefix() final { return "NullLogger"; } + Ice::LoggerPtr cloneWithPrefix(string) final { return shared_from_this(); } }; } diff --git a/python/modules/IcePy/Logger.cpp b/python/modules/IcePy/Logger.cpp index a60787b4e1c..d67ec475c4b 100644 --- a/python/modules/IcePy/Logger.cpp +++ b/python/modules/IcePy/Logger.cpp @@ -86,7 +86,7 @@ IcePy::LoggerWrapper::getPrefix() } Ice::LoggerPtr -IcePy::LoggerWrapper::cloneWithPrefix(const string& prefix) +IcePy::LoggerWrapper::cloneWithPrefix(string prefix) { AdoptThread adoptThread; // Ensure the current thread is able to call into Python. diff --git a/python/modules/IcePy/Logger.h b/python/modules/IcePy/Logger.h index 72cb88fe9ab..778252329f2 100644 --- a/python/modules/IcePy/Logger.h +++ b/python/modules/IcePy/Logger.h @@ -16,17 +16,17 @@ namespace IcePy // // LoggerWrapper delegates to a Python implementation. // - class LoggerWrapper : public Ice::Logger + class LoggerWrapper final : public Ice::Logger { public: LoggerWrapper(PyObject*); - virtual void print(const std::string&); - virtual void trace(const std::string&, const std::string&); - virtual void warning(const std::string&); - virtual void error(const std::string&); - virtual std::string getPrefix(); - virtual Ice::LoggerPtr cloneWithPrefix(const std::string&); + void print(const std::string&) final; + void trace(const std::string&, const std::string&) final; + void warning(const std::string&) final; + void error(const std::string&) final; + std::string getPrefix() final; + Ice::LoggerPtr cloneWithPrefix(std::string) final; PyObject* getObject(); private: diff --git a/swift/src/IceImpl/LoggerWrapperI.h b/swift/src/IceImpl/LoggerWrapperI.h index b69783ce935..b140a7788b5 100644 --- a/swift/src/IceImpl/LoggerWrapperI.h +++ b/swift/src/IceImpl/LoggerWrapperI.h @@ -1,30 +1,28 @@ // Copyright (c) ZeroC, Inc. #import "Convert.h" -class LoggerWrapperI : public Ice::Logger +class LoggerWrapperI final : public Ice::Logger { public: LoggerWrapperI(id logger) : _logger(logger) {} - virtual ~LoggerWrapperI() {} + void print(const std::string& msg) final { [_logger print:toNSString(msg)]; } - virtual void print(const std::string& msg) { [_logger print:toNSString(msg)]; } - - virtual void trace(const std::string& category, const std::string& msg) + void trace(const std::string& category, const std::string& msg) final { [_logger trace:toNSString(category) message:toNSString(msg)]; } - virtual void warning(const std::string& msg) { [_logger warning:toNSString(msg)]; } + void warning(const std::string& msg) final { [_logger warning:toNSString(msg)]; } - virtual void error(const std::string& msg) { [_logger error:toNSString(msg)]; } + void error(const std::string& msg) final { [_logger error:toNSString(msg)]; } - virtual std::shared_ptr cloneWithPrefix(const std::string& prefix) + std::shared_ptr cloneWithPrefix(std::string prefix) final { return std::make_shared([_logger cloneWithPrefix:toNSString(prefix)]); } - virtual std::string getPrefix() { return fromNSString([_logger getPrefix]); } + std::string getPrefix() final { return fromNSString([_logger getPrefix]); } id getLogger() { return _logger; } From e92b038b0c9f9b3c9015f113f7c69083db2a085a Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Wed, 30 Oct 2024 14:58:38 -0400 Subject: [PATCH 3/4] More cleanup --- cpp/include/Ice/StringConverter.h | 2 +- cpp/include/IceBox/Service.h | 18 +----------------- cpp/src/Ice/Service.cpp | 4 +++- cpp/src/Ice/StringConverter.cpp | 4 ++-- 4 files changed, 7 insertions(+), 21 deletions(-) diff --git a/cpp/include/Ice/StringConverter.h b/cpp/include/Ice/StringConverter.h index 590d4841a86..13efea0604d 100644 --- a/cpp/include/Ice/StringConverter.h +++ b/cpp/include/Ice/StringConverter.h @@ -148,7 +148,7 @@ namespace Ice * @param nc The narrow string converter. If null, the UTF-8 string is returned. * @return A native narrow string. */ - ICE_API std::string UTF8ToNative(const std::string& str, const StringConverterPtr& nc); + ICE_API std::string UTF8ToNative(std::string_view str, const StringConverterPtr& nc); #ifdef _WIN32 diff --git a/cpp/include/IceBox/Service.h b/cpp/include/IceBox/Service.h index fc0a61e61a4..a312589a08c 100644 --- a/cpp/include/IceBox/Service.h +++ b/cpp/include/IceBox/Service.h @@ -1,6 +1,4 @@ -// -// Copyright (c) ZeroC, Inc. All rights reserved. -// +// Copyright (c) ZeroC, Inc. #ifndef ICEBOX_SERVICE_H #define ICEBOX_SERVICE_H @@ -8,14 +6,6 @@ #include "Config.h" #include "Ice/Ice.h" -#if defined(__clang__) -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wshadow-field-in-constructor" -#elif defined(__GNUC__) -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wshadow" -#endif - namespace IceBox { /** @@ -61,10 +51,4 @@ namespace IceBox using ServicePtr = std::shared_ptr; } -#if defined(__clang__) -# pragma clang diagnostic pop -#elif defined(__GNUC__) -# pragma GCC diagnostic pop -#endif - #endif diff --git a/cpp/src/Ice/Service.cpp b/cpp/src/Ice/Service.cpp index 6fef219963b..e70c900e74e 100644 --- a/cpp/src/Ice/Service.cpp +++ b/cpp/src/Ice/Service.cpp @@ -120,7 +120,9 @@ namespace class SMEventLoggerIWrapper final : public Ice::Logger { public: - SMEventLoggerIWrapper(const SMEventLoggerPtr& logger, string prefix) : _logger(logger), _prefix(std::move(prefix)) + SMEventLoggerIWrapper(const SMEventLoggerPtr& logger, string prefix) + : _logger(logger), + _prefix(std::move(prefix)) { assert(_logger); } diff --git a/cpp/src/Ice/StringConverter.cpp b/cpp/src/Ice/StringConverter.cpp index 8e246609591..b5c1c7560e5 100644 --- a/cpp/src/Ice/StringConverter.cpp +++ b/cpp/src/Ice/StringConverter.cpp @@ -341,11 +341,11 @@ Ice::nativeToUTF8(string_view str, const Ice::StringConverterPtr& converter) } string -Ice::UTF8ToNative(const string& str, const Ice::StringConverterPtr& converter) +Ice::UTF8ToNative(string_view str, const Ice::StringConverterPtr& converter) { if (!converter || str.empty()) { - return str; + return string{str}; } string tmp; converter->fromUTF8( From 87fb214328bac13dab200797da40d833dec95e4f Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Wed, 30 Oct 2024 17:19:39 -0400 Subject: [PATCH 4/4] Fix review comments --- cpp/src/Ice/LoggerI.cpp | 8 ++++---- cpp/src/Ice/LoggerI.h | 4 ++-- cpp/src/Ice/OSLogLoggerI.cpp | 10 +++++----- cpp/src/Ice/OSLogLoggerI.h | 1 + cpp/src/Ice/SysLoggerI.cpp | 7 +++++-- cpp/src/Ice/SysLoggerI.h | 3 ++- 6 files changed, 19 insertions(+), 14 deletions(-) diff --git a/cpp/src/Ice/LoggerI.cpp b/cpp/src/Ice/LoggerI.cpp index e7c8f22721e..15448a42429 100644 --- a/cpp/src/Ice/LoggerI.cpp +++ b/cpp/src/Ice/LoggerI.cpp @@ -25,10 +25,11 @@ namespace const chrono::minutes retryTimeout = chrono::minutes(5); } -Ice::LoggerI::LoggerI(string prefix, const string& file, bool convert, size_t sizeMax) +Ice::LoggerI::LoggerI(string prefix, string file, bool convert, size_t sizeMax) : _prefix(std::move(prefix)), _convert(convert), _converter(getProcessStringConverter()), + _file(std::move(file)), _sizeMax(sizeMax) { if (!_prefix.empty()) @@ -36,10 +37,9 @@ Ice::LoggerI::LoggerI(string prefix, const string& file, bool convert, size_t si _formattedPrefix = _prefix + ": "; } - if (!file.empty()) + if (!_file.empty()) { - _file = file; - _out.open(IceInternal::streamFilename(file).c_str(), fstream::out | fstream::app); + _out.open(IceInternal::streamFilename(_file).c_str(), fstream::out | fstream::app); if (!_out.is_open()) { throw InitializationException(__FILE__, __LINE__, "FileLogger: cannot open " + _file); diff --git a/cpp/src/Ice/LoggerI.h b/cpp/src/Ice/LoggerI.h index 3d2475076cc..69076a71176 100644 --- a/cpp/src/Ice/LoggerI.h +++ b/cpp/src/Ice/LoggerI.h @@ -15,7 +15,7 @@ namespace Ice class LoggerI final : public Logger { public: - LoggerI(std::string prefix, const std::string&, bool convert = true, std::size_t sizeMax = 0); + LoggerI(std::string prefix, std::string file, bool convert = true, std::size_t sizeMax = 0); ~LoggerI(); void print(const std::string&) final; @@ -34,7 +34,7 @@ namespace Ice const StringConverterPtr _converter; std::ofstream _out; - std::string _file; + const std::string _file; std::size_t _sizeMax; // diff --git a/cpp/src/Ice/OSLogLoggerI.cpp b/cpp/src/Ice/OSLogLoggerI.cpp index f7ffbdcc35d..caaf407a6d4 100644 --- a/cpp/src/Ice/OSLogLoggerI.cpp +++ b/cpp/src/Ice/OSLogLoggerI.cpp @@ -10,10 +10,11 @@ using namespace std; using namespace Ice; -Ice::OSLogLoggerI::OSLogLoggerI(string prefix) : _prefix(std::move(prefix)) +Ice::OSLogLoggerI::OSLogLoggerI(string prefix) + : _prefix(std::move(prefix)), + _subsystem(_prefix.empty() ? "com.zeroc.ice" : "com.zeroc.ice." + _prefix), + _log(os_log_create(_subsystem.c_str(), "")) { - const string subsystem = _prefix.empty() ? "com.zeroc.ice" : "com.zeroc.ice." + _prefix; - _log.reset(os_log_create(subsystem.c_str(), "")); } void @@ -25,8 +26,7 @@ Ice::OSLogLoggerI::print(const string& message) void Ice::OSLogLoggerI::trace(const string& category, const string& message) { - const string subsystem = _prefix.empty() ? "com.zeroc.ice" : "com.zeroc.ice." + _prefix; - IceInternal::UniqueRef log(os_log_create(subsystem.c_str(), category.c_str())); + IceInternal::UniqueRef log(os_log_create(_subsystem.c_str(), category.c_str())); os_log_with_type(log.get(), OS_LOG_TYPE_INFO, "%{public}s.", message.c_str()); } diff --git a/cpp/src/Ice/OSLogLoggerI.h b/cpp/src/Ice/OSLogLoggerI.h index ac5ccadb2d2..019dfd4408b 100644 --- a/cpp/src/Ice/OSLogLoggerI.h +++ b/cpp/src/Ice/OSLogLoggerI.h @@ -27,6 +27,7 @@ namespace Ice private: const std::string _prefix; + const std::string _subsystem; IceInternal::UniqueRef _log; }; } diff --git a/cpp/src/Ice/SysLoggerI.cpp b/cpp/src/Ice/SysLoggerI.cpp index 516284344c1..4dd939ad714 100644 --- a/cpp/src/Ice/SysLoggerI.cpp +++ b/cpp/src/Ice/SysLoggerI.cpp @@ -12,7 +12,7 @@ using namespace std; using namespace Ice; using namespace IceInternal; -Ice::SysLoggerI::SysLoggerI(string prefix, const string& facilityString) : _facility(0), _prefix(std::move(prefix)) +Ice::SysLoggerI::SysLoggerI(string prefix, string_view facilityString) : _facility(0), _prefix(std::move(prefix)) { if (facilityString == "LOG_KERN") { @@ -100,7 +100,10 @@ Ice::SysLoggerI::SysLoggerI(string prefix, const string& facilityString) : _faci } else { - throw InitializationException(__FILE__, __LINE__, "Invalid value for Ice.SyslogFacility: " + facilityString); + throw InitializationException( + __FILE__, + __LINE__, + "Invalid value for Ice.SyslogFacility: " + string{facilityString}); } int logopt = LOG_PID | LOG_CONS; diff --git a/cpp/src/Ice/SysLoggerI.h b/cpp/src/Ice/SysLoggerI.h index e373cd1c297..eec068cd0f0 100644 --- a/cpp/src/Ice/SysLoggerI.h +++ b/cpp/src/Ice/SysLoggerI.h @@ -8,13 +8,14 @@ #include "Ice/Logger.h" #include +#include namespace Ice { class SysLoggerI final : public Logger { public: - SysLoggerI(std::string prefix, const std::string& facilityString); + SysLoggerI(std::string prefix, std::string_view facilityString); SysLoggerI(std::string prefix, int facility); ~SysLoggerI();