clang-tidy fixes for Linux/OpenSSL (#3407)

cpp/include/Ice/Proxy.h

@@ -142,7 +142,7 @@ namespace Ice
          */
         [[nodiscard]] Prx ice_encodingVersion(EncodingVersion version) const
         {
-            return fromReference(asPrx()._encodingVersion(std::move(version)));
+            return fromReference(asPrx()._encodingVersion(version));
         }
 
         /**

cpp/src/Ice/DynamicLibrary.cpp

@@ -144,7 +144,7 @@ IceInternal::DynamicLibrary::loadEntryPoint(const string& entryPoint, bool useIc
 #else
     if (!load(lib))
     {
-        return 0;
+        return nullptr;
     }
 #endif
 

cpp/src/Ice/LocalException.cpp

@@ -34,13 +34,17 @@
 
 #ifdef __GNUC__
 #    if defined(ICE_LIBBACKTRACE)
-#        include <backtrace-supported.h>
-#        include <backtrace.h>
-#        if BACKTRACE_SUPPORTED && BACKTRACE_SUPPORTS_THREADS
-#            include <algorithm>
+#        ifdef __clang__
+#            undef ICE_LIBBACKTRACE // for clang-tidy
 #        else
+#            include <backtrace-supported.h>
+#            include <backtrace.h>
+#            if BACKTRACE_SUPPORTED && BACKTRACE_SUPPORTS_THREADS
+#                include <algorithm>
+#            else
 // It's available but we can't use it - shouldn't happen
-#            undef ICE_LIBBACKTRACE
+#                undef ICE_LIBBACKTRACE
+#            endif
 #        endif
 #    endif
 
@@ -231,18 +235,18 @@ namespace
 
 #ifdef ICE_LIBBACKTRACE
 
-    void handlePcInfoError(void* data, const char* /*msg*/, int /*errnum*/)
+    void handlePcInfoError(void* data, [[maybe_unused]] const char* msg, [[maybe_unused]] int errnum)
     {
         FrameInfo& frameInfo = *reinterpret_cast<FrameInfo*>(data);
-        printFrame(&frameInfo, frameInfo.pc, 0, 0, 0);
+        printFrame(&frameInfo, frameInfo.pc, nullptr, 0, nullptr);
         frameInfo.setByErrorCb = true;
     }
 
     int addFrame(void* sf, uintptr_t pc)
     {
         if (pc != UINTPTR_MAX)
         {
-            vector<void*>* stackFrames = reinterpret_cast<vector<void*>*>(sf);
+            auto* stackFrames = reinterpret_cast<vector<void*>*>(sf);
             stackFrames->push_back(reinterpret_cast<void*>(pc));
             return 0;
         }
@@ -427,7 +431,7 @@ Ice::LocalException::ice_print(ostream& os) const
     // +2 to remove the leading "::" from the type ID.
     os << ice_id() + 2 << ' ' << what();
 
-    string stack = ice_stackTrace();
+    string stack{ice_stackTrace()};
     if (!stack.empty())
     {
         os << "\nstack trace:\n" << stack;
@@ -506,7 +510,7 @@ Ice::LocalException::ice_enableStackTraceCollection()
     if (!bstate)
     {
         // Leaked, as libbacktrace does not provide an API to free this state.
-        bstate = backtrace_create_state(0, 1, ignoreErrorCallback, 0);
+        bstate = backtrace_create_state(nullptr, 1, ignoreErrorCallback, nullptr);
     }
 #elif defined(ICE_BACKTRACE)
     backTraceEnabled = true;

cpp/src/Ice/ObjectAdapterFactory.cpp

@@ -116,7 +116,7 @@ ObjectAdapterPtr
 IceInternal::ObjectAdapterFactory::createObjectAdapter(
     string name, // NOLINT(performance-unnecessary-value-param)
     optional<Ice::RouterPrx> router,
-    optional<SSL::ServerAuthenticationOptions> serverAuthenticationOptions)
+    optional<Ice::SSL::ServerAuthenticationOptions> serverAuthenticationOptions)
 {
     shared_ptr<ObjectAdapterI> adapter;
     {

cpp/src/Ice/Reference.cpp

@@ -1338,7 +1338,8 @@ IceInternal::RoutableReference::getRequestHandler() const
         { handler->setConnection(std::move(connection), compress); },
         [handler](exception_ptr ex) { handler->setException(ex); });
 
-    return handler;
+    return handler; // NOLINT(clang-analyzer-cplusplus.NewDeleteLeaks)
+    // Looks like a clang-tidy bug. See https://github.com/llvm/llvm-project/issues/55219
 }
 
 const BatchRequestQueuePtr&

cpp/src/Ice/SSL/OpenSSLEngine.cpp

@@ -36,7 +36,7 @@ extern "C"
 {
     int Ice_SSL_opensslPasswordCallback(char* buf, int size, int /*flag*/, void* userData)
     {
-        OpenSSL::SSLEngine* p = reinterpret_cast<OpenSSL::SSLEngine*>(userData);
+        auto* p = reinterpret_cast<OpenSSL::SSLEngine*>(userData);
         assert(p);
         string passwd = p->password();
         int sz = static_cast<int>(passwd.size());
@@ -47,9 +47,9 @@ extern "C"
         strncpy(buf, passwd.c_str(), sz);
         buf[sz] = '\0';
 
-        for (string::iterator i = passwd.begin(); i != passwd.end(); ++i)
+        for (auto& character : passwd)
         {
-            *i = '\0';
+            character = '\0';
         }
 
         return sz;
@@ -176,7 +176,7 @@ OpenSSL::SSLEngine::initialize()
             int success = 0;
 
             const unsigned char* b = reinterpret_cast<unsigned char*>(&buffer[0]);
-            PKCS12* p12 = d2i_PKCS12(0, &b, static_cast<long>(buffer.size()));
+            PKCS12* p12 = d2i_PKCS12(nullptr, &b, static_cast<long>(buffer.size()));
             if (p12)
             {
                 EVP_PKEY* key = nullptr;
@@ -225,7 +225,7 @@ OpenSSL::SSLEngine::initialize()
                         // Pop each cert from the stack so we can free the stack later.
                         // The CTX destruction will take care of the certificates
                         X509* c = nullptr;
-                        while ((c = sk_X509_pop(chain)) != 0)
+                        while ((c = sk_X509_pop(chain)))
                         {
                             if (!SSL_CTX_add_extra_chain_cert(_ctx, c))
                             {
@@ -429,7 +429,7 @@ OpenSSL::SSLEngine::createClientAuthenticationOptions(const std::string&) const
                     }
                 }
             }
-            SSL_set_verify(ssl, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, 0);
+            SSL_set_verify(ssl, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nullptr);
         },
         .serverCertificateValidationCallback =
             [this](bool ok, X509_STORE_CTX* ctx, const Ice::SSL::ConnectionInfoPtr& info)
@@ -464,7 +464,7 @@ OpenSSL::SSLEngine::createServerAuthenticationOptions() const
                     sslVerifyMode = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
                     break;
             }
-            SSL_set_verify(ssl, sslVerifyMode, 0);
+            SSL_set_verify(ssl, sslVerifyMode, nullptr);
         },
         .clientCertificateValidationCallback =
             [this](bool ok, X509_STORE_CTX* ctx, const Ice::SSL::ConnectionInfoPtr& info)

cpp/src/Ice/SSL/OpenSSLTransceiverI.cpp

@@ -26,8 +26,8 @@ extern "C"
 {
     int Ice_SSL_opensslVerifyCallback(int ok, X509_STORE_CTX* ctx)
     {
-        ::SSL* ssl = reinterpret_cast<::SSL*>(X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
-        OpenSSL::TransceiverI* p = reinterpret_cast<OpenSSL::TransceiverI*>(SSL_get_ex_data(ssl, 0));
+        auto* ssl = reinterpret_cast<::SSL*>(X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
+        auto* p = reinterpret_cast<OpenSSL::TransceiverI*>(SSL_get_ex_data(ssl, 0));
         return p->verifyCallback(ok, ctx);
     }
 }
@@ -257,7 +257,7 @@ OpenSSL::TransceiverI::initialize(IceInternal::Buffer& readBuffer, IceInternal::
         else
         {
             out << "cipher = " << SSL_CIPHER_get_name(cipher) << "\n";
-            out << "bits = " << SSL_CIPHER_get_bits(cipher, 0) << "\n";
+            out << "bits = " << SSL_CIPHER_get_bits(cipher, nullptr) << "\n";
             out << "protocol = " << SSL_get_version(_ssl) << "\n";
         }
         out << toString();
@@ -678,16 +678,16 @@ OpenSSL::TransceiverI::verifyCallback(int ok, X509_STORE_CTX* ctx)
 }
 
 OpenSSL::TransceiverI::TransceiverI(
-    const InstancePtr& instance,
-    const IceInternal::TransceiverPtr& delegate,
-    const string& adapterName,
+    InstancePtr instance,
+    IceInternal::TransceiverPtr delegate,
+    string adapterName,
     const ServerAuthenticationOptions& serverAuthenticationOptions)
-    : _instance(instance),
-      _engine(dynamic_pointer_cast<OpenSSL::SSLEngine>(instance->engine())),
+    : _instance(std::move(instance)),
+      _engine(dynamic_pointer_cast<OpenSSL::SSLEngine>(_instance->engine())),
       _host(""),
-      _adapterName(adapterName),
+      _adapterName(std::move(adapterName)),
       _incoming(true),
-      _delegate(delegate),
+      _delegate(std::move(delegate)),
       _connected(false),
       _peerCertificate(nullptr),
       _ssl(nullptr),
@@ -709,16 +709,16 @@ OpenSSL::TransceiverI::TransceiverI(
 }
 
 OpenSSL::TransceiverI::TransceiverI(
-    const InstancePtr& instance,
-    const IceInternal::TransceiverPtr& delegate,
-    const string& host,
+    InstancePtr instance,
+    IceInternal::TransceiverPtr delegate,
+    string host,
     const ClientAuthenticationOptions& clientAuthenticationOptions)
-    : _instance(instance),
-      _engine(dynamic_pointer_cast<OpenSSL::SSLEngine>(instance->engine())),
-      _host(host),
+    : _instance(std::move(instance)),
+      _engine(dynamic_pointer_cast<OpenSSL::SSLEngine>(_instance->engine())),
+      _host(std::move(host)),
       _adapterName(""),
       _incoming(false),
-      _delegate(delegate),
+      _delegate(std::move(delegate)),
       _connected(false),
       _peerCertificate(nullptr),
       _ssl(nullptr),

cpp/src/Ice/SSL/OpenSSLTransceiverI.h

@@ -15,23 +15,23 @@
 
 #include <openssl/ssl.h>
 
-typedef struct ssl_st SSL;
-typedef struct bio_st BIO;
+using SSL = struct ssl_st;
+using BIO = struct bio_st;
 
 namespace Ice::SSL::OpenSSL
 {
     class TransceiverI final : public IceInternal::Transceiver
     {
     public:
         TransceiverI(
-            const InstancePtr&,
-            const IceInternal::TransceiverPtr&,
-            const std::string&,
+            InstancePtr,
+            IceInternal::TransceiverPtr,
+            std::string,
             const Ice::SSL::ServerAuthenticationOptions&);
         TransceiverI(
-            const InstancePtr&,
-            const IceInternal::TransceiverPtr&,
-            const std::string&,
+            InstancePtr,
+            IceInternal::TransceiverPtr,
+            std::string,
             const Ice::SSL::ClientAuthenticationOptions&);
 
         ~TransceiverI();

cpp/src/Ice/SSL/SSLUtil.cpp

@@ -480,7 +480,7 @@ namespace
     vector<pair<int, string>> convertGeneralNames(GENERAL_NAMES* gens)
     {
         vector<pair<int, string>> alt;
-        if (gens == 0)
+        if (gens == nullptr)
         {
             return alt;
         }
@@ -639,7 +639,7 @@ vector<pair<int, string>>
 Ice::SSL::getSubjectAltNames(X509* certificate)
 {
     return convertGeneralNames(
-        reinterpret_cast<GENERAL_NAMES*>(X509_get_ext_d2i(certificate, NID_subject_alt_name, 0, 0)));
+        reinterpret_cast<GENERAL_NAMES*>(X509_get_ext_d2i(certificate, NID_subject_alt_name, nullptr, nullptr)));
 }
 
 string

cpp/src/Ice/SSL/SSLUtil.h

@@ -92,7 +92,7 @@ namespace Ice::SSL
     public:
         ScopedCertificate(X509* certificate) : _certificate(certificate) {}
         ~ScopedCertificate();
-        X509* get() const { return _certificate; }
+        [[nodiscard]] X509* get() const { return _certificate; }
 
     private:
         X509* _certificate;

cpp/src/Ice/Selector.cpp

@@ -190,7 +190,7 @@ Selector::Selector(InstancePtr instance) : _instance(std::move(instance)), _inte
 
     epoll_event event;
     memset(&event, 0, sizeof(epoll_event));
-    event.data.ptr = 0;
+    event.data.ptr = nullptr;
     event.events = EPOLLIN;
     if (epoll_ctl(_queueFd, EPOLL_CTL_ADD, _fdIntrRead, &event) != 0)
     {
@@ -296,8 +296,8 @@ Selector::enable(EventHandler* handler, SocketOperation status)
     {
 #    if defined(ICE_USE_EPOLL)
         SOCKET fd = nativeInfo->fd();
-        SocketOperation previous = static_cast<SocketOperation>(handler->_registered & ~(handler->_disabled | status));
-        SocketOperation newStatus = static_cast<SocketOperation>(handler->_registered & ~handler->_disabled);
+        auto previous = static_cast<SocketOperation>(handler->_registered & ~(handler->_disabled | status));
+        auto newStatus = static_cast<SocketOperation>(handler->_registered & ~handler->_disabled);
         epoll_event event;
         memset(&event, 0, sizeof(epoll_event));
         event.data.ptr = handler;
@@ -351,7 +351,7 @@ Selector::disable(EventHandler* handler, SocketOperation status)
     {
 #    if defined(ICE_USE_EPOLL)
         SOCKET fd = nativeInfo->fd();
-        SocketOperation newStatus = static_cast<SocketOperation>(handler->_registered & ~handler->_disabled);
+        auto newStatus = static_cast<SocketOperation>(handler->_registered & ~handler->_disabled);
         epoll_event event;
         memset(&event, 0, sizeof(epoll_event));
         event.data.ptr = handler;

cpp/src/Ice/WSEndpoint.cpp

@@ -234,7 +234,7 @@ IceInternal::WSEndpoint::connectorsAsync(
 AcceptorPtr
 IceInternal::WSEndpoint::acceptor(
     const string& adapterName,
-    const optional<SSL::ServerAuthenticationOptions>& serverAuthenticationOptions) const
+    const optional<Ice::SSL::ServerAuthenticationOptions>& serverAuthenticationOptions) const
 {
     AcceptorPtr acceptor = _delegate->acceptor(adapterName, serverAuthenticationOptions);
     return make_shared<WSAcceptor>(const_cast<WSEndpoint*>(this)->shared_from_this(), _instance, acceptor);