From d885dd5b642807d0587acad43668cfccfdf06d1e Mon Sep 17 00:00:00 2001 From: comex Date: Sun, 25 Jun 2023 19:23:23 -0700 Subject: [PATCH] PR feedback + constification --- src/core/hle/service/sockets/bsd.cpp | 8 ++-- src/core/hle/service/sockets/nsd.cpp | 6 +-- src/core/hle/service/sockets/sfdnsres.cpp | 22 ++++----- src/core/hle/service/ssl/ssl.cpp | 20 ++++----- src/core/hle/service/ssl/ssl_backend.h | 4 +- src/core/hle/service/ssl/ssl_backend_none.cpp | 3 +- .../hle/service/ssl/ssl_backend_openssl.cpp | 14 +++--- .../hle/service/ssl/ssl_backend_schannel.cpp | 45 ++++++++++--------- 8 files changed, 62 insertions(+), 60 deletions(-) diff --git a/src/core/hle/service/sockets/bsd.cpp b/src/core/hle/service/sockets/bsd.cpp index 6677689dc..6034cc0b5 100644 --- a/src/core/hle/service/sockets/bsd.cpp +++ b/src/core/hle/service/sockets/bsd.cpp @@ -270,10 +270,10 @@ void BSD::GetSockOpt(HLERequestContext& ctx) { std::vector optval(ctx.GetWriteBufferSize()); - LOG_WARNING(Service, "called. fd={} level={} optname=0x{:x} len=0x{:x}", fd, level, optname, - optval.size()); + LOG_DEBUG(Service, "called. fd={} level={} optname=0x{:x} len=0x{:x}", fd, level, optname, + optval.size()); - Errno err = GetSockOptImpl(fd, level, optname, optval); + const Errno err = GetSockOptImpl(fd, level, optname, optval); ctx.WriteBuffer(optval); @@ -447,7 +447,7 @@ void BSD::DuplicateSocket(HLERequestContext& ctx) { const s32 fd = rp.Pop(); [[maybe_unused]] const u64 unused = rp.Pop(); - Common::Expected res = DuplicateSocketImpl(fd); + Expected res = DuplicateSocketImpl(fd); IPC::ResponseBuilder rb{ctx, 4}; rb.Push(ResultSuccess); rb.Push(res.value_or(0)); // ret diff --git a/src/core/hle/service/sockets/nsd.cpp b/src/core/hle/service/sockets/nsd.cpp index 22c3a31a0..0dfb0f166 100644 --- a/src/core/hle/service/sockets/nsd.cpp +++ b/src/core/hle/service/sockets/nsd.cpp @@ -49,7 +49,7 @@ static ResultVal ResolveImpl(const std::string& fqdn_in) { // The real implementation makes various substitutions. // For now we just return the string as-is, which is good enough when not // connecting to real Nintendo servers. - LOG_WARNING(Service, "(STUBBED) called({})", fqdn_in); + LOG_WARNING(Service, "(STUBBED) called, fqdn_in={}", fqdn_in); return fqdn_in; } @@ -69,7 +69,7 @@ void NSD::Resolve(HLERequestContext& ctx) { const std::string fqdn_in = Common::StringFromBuffer(ctx.ReadBuffer(0)); std::array fqdn_out{}; - Result res = ResolveCommon(fqdn_in, fqdn_out); + const Result res = ResolveCommon(fqdn_in, fqdn_out); ctx.WriteBuffer(fqdn_out); IPC::ResponseBuilder rb{ctx, 2}; @@ -80,7 +80,7 @@ void NSD::ResolveEx(HLERequestContext& ctx) { const std::string fqdn_in = Common::StringFromBuffer(ctx.ReadBuffer(0)); std::array fqdn_out; - Result res = ResolveCommon(fqdn_in, fqdn_out); + const Result res = ResolveCommon(fqdn_in, fqdn_out); if (res.IsError()) { IPC::ResponseBuilder rb{ctx, 2}; diff --git a/src/core/hle/service/sockets/sfdnsres.cpp b/src/core/hle/service/sockets/sfdnsres.cpp index c5eaec920..45f0f526a 100644 --- a/src/core/hle/service/sockets/sfdnsres.cpp +++ b/src/core/hle/service/sockets/sfdnsres.cpp @@ -88,15 +88,15 @@ static Errno GetAddrInfoErrorToErrno(GetAddrInfoError result) { template static void Append(std::vector& vec, T t) { - size_t off = vec.size(); - vec.resize(off + sizeof(T)); - std::memcpy(vec.data() + off, &t, sizeof(T)); + const size_t offset = vec.size(); + vec.resize(offset + sizeof(T)); + std::memcpy(vec.data() + offset, &t, sizeof(T)); } static void AppendNulTerminated(std::vector& vec, std::string_view str) { - size_t off = vec.size(); - vec.resize(off + str.size() + 1); - std::memmove(vec.data() + off, str.data(), str.size()); + const size_t offset = vec.size(); + vec.resize(offset + str.size() + 1); + std::memmove(vec.data() + offset, str.data(), str.size()); } // We implement gethostbyname using the host's getaddrinfo rather than the @@ -154,8 +154,8 @@ static std::pair GetHostByNameRequestImpl(HLERequestConte return {0, Translate(res.error())}; } - std::vector data = SerializeAddrInfoAsHostEnt(res.value(), host); - u32 data_size = static_cast(data.size()); + const std::vector data = SerializeAddrInfoAsHostEnt(res.value(), host); + const u32 data_size = static_cast(data.size()); ctx.WriteBuffer(data, 0); return {data_size, GetAddrInfoError::SUCCESS}; @@ -243,7 +243,7 @@ static std::pair GetAddrInfoRequestImpl(HLERequestContext std::optional service = std::nullopt; if (ctx.CanReadBuffer(1)) { - std::span service_buffer = ctx.ReadBuffer(1); + const std::span service_buffer = ctx.ReadBuffer(1); service = Common::StringFromBuffer(service_buffer); } @@ -254,8 +254,8 @@ static std::pair GetAddrInfoRequestImpl(HLERequestContext return {0, Translate(res.error())}; } - std::vector data = SerializeAddrInfo(res.value(), host); - u32 data_size = static_cast(data.size()); + const std::vector data = SerializeAddrInfo(res.value(), host); + const u32 data_size = static_cast(data.size()); ctx.WriteBuffer(data, 0); return {data_size, GetAddrInfoError::SUCCESS}; diff --git a/src/core/hle/service/ssl/ssl.cpp b/src/core/hle/service/ssl/ssl.cpp index a3b54c7f0..5638dd693 100644 --- a/src/core/hle/service/ssl/ssl.cpp +++ b/src/core/hle/service/ssl/ssl.cpp @@ -114,7 +114,7 @@ public: ~ISslConnection() { shared_data_->connection_count--; if (fd_to_close_.has_value()) { - s32 fd = *fd_to_close_; + const s32 fd = *fd_to_close_; if (!do_not_close_socket_) { LOG_ERROR(Service_SSL, "do_not_close_socket was changed after setting socket; is this right?"); @@ -123,7 +123,7 @@ public: if (bsd) { auto err = bsd->CloseImpl(fd); if (err != Service::Sockets::Errno::SUCCESS) { - LOG_ERROR(Service_SSL, "failed to close duplicated socket: {}", err); + LOG_ERROR(Service_SSL, "Failed to close duplicated socket: {}", err); } } } @@ -151,7 +151,7 @@ private: if (do_not_close_socket_) { auto res = bsd->DuplicateSocketImpl(fd); if (!res.has_value()) { - LOG_ERROR(Service_SSL, "failed to duplicate socket"); + LOG_ERROR(Service_SSL, "Failed to duplicate socket with fd {}", fd); return ResultInvalidSocket; } fd = *res; @@ -171,7 +171,7 @@ private: } Result SetHostNameImpl(const std::string& hostname) { - LOG_DEBUG(Service_SSL, "SetHostNameImpl({})", hostname); + LOG_DEBUG(Service_SSL, "called. hostname={}", hostname); ASSERT(!did_handshake_); Result res = backend_->SetHostName(hostname); if (res == ResultSuccess) { @@ -191,9 +191,9 @@ private: ASSERT(mode == IoMode::Blocking || mode == IoMode::NonBlocking); ASSERT_OR_EXECUTE(socket_, { return ResultNoSocket; }); - bool non_block = mode == IoMode::NonBlocking; - Network::Errno e = socket_->SetNonBlock(non_block); - if (e != Network::Errno::SUCCESS) { + const bool non_block = mode == IoMode::NonBlocking; + const Network::Errno error = socket_->SetNonBlock(non_block); + if (error != Network::Errno::SUCCESS) { LOG_ERROR(Service_SSL, "Failed to set native socket non-block flag to {}", non_block); } return ResultSuccess; @@ -307,13 +307,13 @@ private: } void DoHandshakeGetServerCert(HLERequestContext& ctx) { - Result res = DoHandshakeImpl(); + const Result res = DoHandshakeImpl(); u32 certs_count = 0; u32 certs_size = 0; if (res == ResultSuccess) { auto certs = backend_->GetServerCerts(); if (certs.Succeeded()) { - std::vector certs_buf = SerializeServerCerts(*certs); + const std::vector certs_buf = SerializeServerCerts(*certs); ctx.WriteBuffer(certs_buf); certs_count = static_cast(certs->size()); certs_size = static_cast(certs_buf.size()); @@ -377,7 +377,7 @@ private: get_server_cert_chain_ = static_cast(parameters.value); break; default: - LOG_WARNING(Service_SSL, "unrecognized option={}, value={}", parameters.option, + LOG_WARNING(Service_SSL, "Unknown option={}, value={}", parameters.option, parameters.value); } diff --git a/src/core/hle/service/ssl/ssl_backend.h b/src/core/hle/service/ssl/ssl_backend.h index 0dd8d9118..25c16bcc1 100644 --- a/src/core/hle/service/ssl/ssl_backend.h +++ b/src/core/hle/service/ssl/ssl_backend.h @@ -23,11 +23,11 @@ constexpr Result ResultInvalidSocket{ErrorModule::SSLSrv, 106}; constexpr Result ResultTimeout{ErrorModule::SSLSrv, 205}; constexpr Result ResultInternalError{ErrorModule::SSLSrv, 999}; // made up -constexpr Result ResultWouldBlock{ErrorModule::SSLSrv, 204}; -// ^ ResultWouldBlock is returned from Read and Write, and oddly, DoHandshake, +// ResultWouldBlock is returned from Read and Write, and oddly, DoHandshake, // with no way in the latter case to distinguish whether the client should poll // for read or write. The one official client I've seen handles this by always // polling for read (with a timeout). +constexpr Result ResultWouldBlock{ErrorModule::SSLSrv, 204}; class SSLConnectionBackend { public: diff --git a/src/core/hle/service/ssl/ssl_backend_none.cpp b/src/core/hle/service/ssl/ssl_backend_none.cpp index eb01561e2..f2f0ef706 100644 --- a/src/core/hle/service/ssl/ssl_backend_none.cpp +++ b/src/core/hle/service/ssl/ssl_backend_none.cpp @@ -8,7 +8,8 @@ namespace Service::SSL { ResultVal> CreateSSLConnectionBackend() { - LOG_ERROR(Service_SSL, "No SSL backend on this platform"); + LOG_ERROR(Service_SSL, + "Can't create SSL connection because no SSL backend is available on this platform"); return ResultInternalError; } diff --git a/src/core/hle/service/ssl/ssl_backend_openssl.cpp b/src/core/hle/service/ssl/ssl_backend_openssl.cpp index bc797b76b..e7d5801fd 100644 --- a/src/core/hle/service/ssl/ssl_backend_openssl.cpp +++ b/src/core/hle/service/ssl/ssl_backend_openssl.cpp @@ -90,15 +90,15 @@ public: Result DoHandshake() override { SSL_set_verify_result(ssl_, X509_V_OK); - int ret = SSL_do_handshake(ssl_); - long verify_result = SSL_get_verify_result(ssl_); + const int ret = SSL_do_handshake(ssl_); + const long verify_result = SSL_get_verify_result(ssl_); if (verify_result != X509_V_OK) { LOG_ERROR(Service_SSL, "SSL cert verification failed because: {}", X509_verify_cert_error_string(verify_result)); return CheckOpenSSLErrors(); } if (ret <= 0) { - int ssl_err = SSL_get_error(ssl_, ret); + const int ssl_err = SSL_get_error(ssl_, ret); if (ssl_err == SSL_ERROR_ZERO_RETURN || (ssl_err == SSL_ERROR_SYSCALL && got_read_eof_)) { LOG_ERROR(Service_SSL, "SSL handshake failed because server hung up"); @@ -110,18 +110,18 @@ public: ResultVal Read(std::span data) override { size_t actual; - int ret = SSL_read_ex(ssl_, data.data(), data.size(), &actual); + const int ret = SSL_read_ex(ssl_, data.data(), data.size(), &actual); return HandleReturn("SSL_read_ex", actual, ret); } ResultVal Write(std::span data) override { size_t actual; - int ret = SSL_write_ex(ssl_, data.data(), data.size(), &actual); + const int ret = SSL_write_ex(ssl_, data.data(), data.size(), &actual); return HandleReturn("SSL_write_ex", actual, ret); } ResultVal HandleReturn(const char* what, size_t actual, int ret) { - int ssl_err = SSL_get_error(ssl_, ret); + const int ssl_err = SSL_get_error(ssl_, ret); CheckOpenSSLErrors(); switch (ssl_err) { case SSL_ERROR_NONE: @@ -255,7 +255,7 @@ public: ResultVal> CreateSSLConnectionBackend() { auto conn = std::make_unique(); - Result res = conn->Init(); + const Result res = conn->Init(); if (res.IsFailure()) { return res; } diff --git a/src/core/hle/service/ssl/ssl_backend_schannel.cpp b/src/core/hle/service/ssl/ssl_backend_schannel.cpp index d293adcf7..775d5cc07 100644 --- a/src/core/hle/service/ssl/ssl_backend_schannel.cpp +++ b/src/core/hle/service/ssl/ssl_backend_schannel.cpp @@ -38,7 +38,7 @@ static void OneTimeInit() { // certificate, and presenting one to some arbitrary server // might be a privacy concern? Who knows, though. - SECURITY_STATUS ret = + const SECURITY_STATUS ret = AcquireCredentialsHandle(nullptr, const_cast(UNISP_NAME), SECPKG_CRED_OUTBOUND, nullptr, &schannel_cred, nullptr, nullptr, &cred_handle, nullptr); if (ret != SEC_E_OK) { @@ -121,14 +121,14 @@ public: } Result FillCiphertextReadBuf() { - size_t fill_size = read_buf_fill_size_ ? read_buf_fill_size_ : 4096; + const size_t fill_size = read_buf_fill_size_ ? read_buf_fill_size_ : 4096; read_buf_fill_size_ = 0; // This unnecessarily zeroes the buffer; oh well. - size_t offset = ciphertext_read_buf_.size(); + const size_t offset = ciphertext_read_buf_.size(); ASSERT_OR_EXECUTE(offset + fill_size >= offset, { return ResultInternalError; }); ciphertext_read_buf_.resize(offset + fill_size, 0); - auto read_span = std::span(ciphertext_read_buf_).subspan(offset, fill_size); - auto [actual, err] = socket_->Recv(0, read_span); + const auto read_span = std::span(ciphertext_read_buf_).subspan(offset, fill_size); + const auto [actual, err] = socket_->Recv(0, read_span); switch (err) { case Network::Errno::SUCCESS: ASSERT(static_cast(actual) <= fill_size); @@ -147,7 +147,7 @@ public: // Returns success if the write buffer has been completely emptied. Result FlushCiphertextWriteBuf() { while (!ciphertext_write_buf_.empty()) { - auto [actual, err] = socket_->Send(ciphertext_write_buf_, 0); + const auto [actual, err] = socket_->Send(ciphertext_write_buf_, 0); switch (err) { case Network::Errno::SUCCESS: ASSERT(static_cast(actual) <= ciphertext_write_buf_.size()); @@ -165,9 +165,10 @@ public: } Result CallInitializeSecurityContext() { - unsigned long req = ISC_REQ_ALLOCATE_MEMORY | ISC_REQ_CONFIDENTIALITY | ISC_REQ_INTEGRITY | - ISC_REQ_REPLAY_DETECT | ISC_REQ_SEQUENCE_DETECT | ISC_REQ_STREAM | - ISC_REQ_USE_SUPPLIED_CREDS; + const unsigned long req = ISC_REQ_ALLOCATE_MEMORY | ISC_REQ_CONFIDENTIALITY | + ISC_REQ_INTEGRITY | ISC_REQ_REPLAY_DETECT | + ISC_REQ_SEQUENCE_DETECT | ISC_REQ_STREAM | + ISC_REQ_USE_SUPPLIED_CREDS; unsigned long attr; // https://learn.microsoft.com/en-us/windows/win32/secauthn/initializesecuritycontext--schannel std::array input_buffers{{ @@ -219,7 +220,7 @@ public: ciphertext_read_buf_.size()); } - SECURITY_STATUS ret = + const SECURITY_STATUS ret = InitializeSecurityContextA(&cred_handle, initial_call_done ? &ctxt_ : nullptr, // Caller ensured we have set a hostname: const_cast(hostname_.value().c_str()), req, @@ -231,15 +232,15 @@ public: nullptr); // ptsExpiry if (output_buffers[0].pvBuffer) { - std::span span(static_cast(output_buffers[0].pvBuffer), - output_buffers[0].cbBuffer); + const std::span span(static_cast(output_buffers[0].pvBuffer), + output_buffers[0].cbBuffer); ciphertext_write_buf_.insert(ciphertext_write_buf_.end(), span.begin(), span.end()); FreeContextBuffer(output_buffers[0].pvBuffer); } if (output_buffers[1].pvBuffer) { - std::span span(static_cast(output_buffers[1].pvBuffer), - output_buffers[1].cbBuffer); + const std::span span(static_cast(output_buffers[1].pvBuffer), + output_buffers[1].cbBuffer); // The documentation doesn't explain what format this data is in. LOG_DEBUG(Service_SSL, "Got a {}-byte alert buffer: {}", span.size(), Common::HexToString(span)); @@ -280,7 +281,7 @@ public: } Result GrabStreamSizes() { - SECURITY_STATUS ret = + const SECURITY_STATUS ret = QueryContextAttributes(&ctxt_, SECPKG_ATTR_STREAM_SIZES, &stream_sizes_); if (ret != SEC_E_OK) { LOG_ERROR(Service_SSL, "QueryContextAttributes(SECPKG_ATTR_STREAM_SIZES) failed: {}", @@ -301,7 +302,7 @@ public: } while (1) { if (!cleartext_read_buf_.empty()) { - size_t read_size = std::min(cleartext_read_buf_.size(), data.size()); + const size_t read_size = std::min(cleartext_read_buf_.size(), data.size()); std::memcpy(data.data(), cleartext_read_buf_.data(), read_size); cleartext_read_buf_.erase(cleartext_read_buf_.begin(), cleartext_read_buf_.begin() + read_size); @@ -366,7 +367,7 @@ public: return ResultInternalError; } } - Result r = FillCiphertextReadBuf(); + const Result r = FillCiphertextReadBuf(); if (r != ResultSuccess) { return r; } @@ -430,7 +431,7 @@ public: .pBuffers = buffers.data(), }; - SECURITY_STATUS ret = EncryptMessage(&ctxt_, /*fQOP*/ 0, &desc, /*MessageSeqNo*/ 0); + const SECURITY_STATUS ret = EncryptMessage(&ctxt_, /*fQOP*/ 0, &desc, /*MessageSeqNo*/ 0); if (ret != SEC_E_OK) { LOG_ERROR(Service_SSL, "EncryptMessage failed: {}", Common::NativeErrorToString(ret)); return ResultInternalError; @@ -445,19 +446,19 @@ public: } ResultVal WriteAlreadyEncryptedData() { - Result r = FlushCiphertextWriteBuf(); + const Result r = FlushCiphertextWriteBuf(); if (r != ResultSuccess) { return r; } // write buf is empty - size_t cleartext_bytes_written = cleartext_write_buf_.size(); + const size_t cleartext_bytes_written = cleartext_write_buf_.size(); cleartext_write_buf_.clear(); return cleartext_bytes_written; } ResultVal>> GetServerCerts() override { PCCERT_CONTEXT returned_cert = nullptr; - SECURITY_STATUS ret = + const SECURITY_STATUS ret = QueryContextAttributes(&ctxt_, SECPKG_ATTR_REMOTE_CERT_CONTEXT, &returned_cert); if (ret != SEC_E_OK) { LOG_ERROR(Service_SSL, @@ -527,7 +528,7 @@ public: ResultVal> CreateSSLConnectionBackend() { auto conn = std::make_unique(); - Result res = conn->Init(); + const Result res = conn->Init(); if (res.IsFailure()) { return res; }