From ce5c3419c32930456718390e1c9e7c05bb8cee12 Mon Sep 17 00:00:00 2001 From: Julien Letessier Date: Mon, 19 Jan 2026 10:21:53 +0100 Subject: [PATCH] Fix critical lint issues and Linux build errors --- src/Buffer.cpp | 5 ++-- src/Conf.h | 5 ++++ src/ConnectConnectionPool.cpp | 21 ++++++++++++- src/Handler.cpp | 56 +++++++++++++++++++++-------------- src/RequestParser.cpp | 10 +++++-- src/SentinelServerPool.cpp | 1 + src/ServerPool.h | 3 +- 7 files changed, 72 insertions(+), 29 deletions(-) diff --git a/src/Buffer.cpp b/src/Buffer.cpp index 4f80ae8..d0463de 100644 --- a/src/Buffer.cpp +++ b/src/Buffer.cpp @@ -199,12 +199,13 @@ Buffer* Segment::fset(Buffer* buf, const char* fmt, ...) va_list ap; va_start(ap, fmt); try { - return vfset(buf, fmt, ap); + Buffer* result = vfset(buf, fmt, ap); + va_end(ap); + return result; } catch (...) { va_end(ap); throw; } - return nullptr; } Buffer* Segment::vfset(Buffer* buf, const char* fmt, va_list ap) diff --git a/src/Conf.h b/src/Conf.h index e3fbc56..8c6a738 100644 --- a/src/Conf.h +++ b/src/Conf.h @@ -75,6 +75,11 @@ struct StandaloneServerPoolConf : public ServerPoolConf std::vector groups; }; +// SentinelServerPool uses the same configuration structure as StandaloneServerPool +struct SentinelServerPoolConf : public StandaloneServerPoolConf +{ +}; + struct ReadPolicyConf { std::string name; diff --git a/src/ConnectConnectionPool.cpp b/src/ConnectConnectionPool.cpp index cedac49..f3a8e70 100644 --- a/src/ConnectConnectionPool.cpp +++ b/src/ConnectConnectionPool.cpp @@ -143,6 +143,14 @@ ConnectConnection* ConnectConnectionPool::getPrivateConnection(int db) } if (c->fd() < 0) { if (mServ->fail()) { + // Free the connection if it was newly created before returning + if (needInit) { + ConnectConnectionAlloc::destroy(c); + { + std::lock_guard lck(mStatsMtx); + --mStats.connections; + } + } return nullptr; } c->reopen(); @@ -155,7 +163,18 @@ ConnectConnection* ConnectConnectionPool::getPrivateConnection(int db) ccl.push_back(c); return nullptr; } - return mServ->fail() ? nullptr : c; + if (mServ->fail()) { + // Free the connection if it was newly created before returning + if (needInit) { + ConnectConnectionAlloc::destroy(c); + { + std::lock_guard lck(mStatsMtx); + --mStats.connections; + } + } + return nullptr; + } + return c; } void ConnectConnectionPool::putPrivateConnection(ConnectConnection* s) diff --git a/src/Handler.cpp b/src/Handler.cpp index f75105d..093538d 100644 --- a/src/Handler.cpp +++ b/src/Handler.cpp @@ -670,7 +670,9 @@ bool Handler::preHandleRequest(Request* req, const String& key) int db = -1; if (key.toInt(db)) { if (db >= 0 && db < mProxy->serverPool()->dbNum()) { - c->setDb(db); + if (c) { + c->setDb(db); + } directResponse(req, Response::Ok); return true; } @@ -1507,26 +1509,32 @@ void Handler::innerResponse(ConnectConnection* s, Request* req, Response* res) break; case Command::AuthServ: if (!res->isOk()) { - s->setStatus(ConnectConnection::LogicError); - addPostEvent(s, Multiplexor::ErrorEvent); - logWarn("h %d s %s %d auth fail", - id(), s->peer(), s->fd()); + if (s) { + s->setStatus(ConnectConnection::LogicError); + addPostEvent(s, Multiplexor::ErrorEvent); + logWarn("h %d s %s %d auth fail", + id(), s->peer(), s->fd()); + } } break; case Command::Readonly: if (!res->isOk()) { - s->setStatus(ConnectConnection::LogicError); - addPostEvent(s, Multiplexor::ErrorEvent); - logWarn("h %d s %s %d readonly fail", - id(), s->peer(), s->fd()); + if (s) { + s->setStatus(ConnectConnection::LogicError); + addPostEvent(s, Multiplexor::ErrorEvent); + logWarn("h %d s %s %d readonly fail", + id(), s->peer(), s->fd()); + } } break; case Command::SelectServ: if (!res->isOk()) { - s->setStatus(ConnectConnection::LogicError); - addPostEvent(s, Multiplexor::ErrorEvent); - logWarn("h %d s %s %d db select %d fail", - id(), s->peer(), s->fd(), s->db()); + if (s) { + s->setStatus(ConnectConnection::LogicError); + addPostEvent(s, Multiplexor::ErrorEvent); + logWarn("h %d s %s %d db select %d fail", + id(), s->peer(), s->fd(), s->db()); + } } break; case Command::ClusterNodes: @@ -1537,18 +1545,22 @@ void Handler::innerResponse(ConnectConnection* s, Request* req, Response* res) break; case Command::UnwatchServ: if (!res->isOk()) { - s->setStatus(ConnectConnection::LogicError); - addPostEvent(s, Multiplexor::ErrorEvent); - logWarn("h %d s %s %d unwatch fail", - id(), s->peer(), s->fd(), s->db()); + if (s) { + s->setStatus(ConnectConnection::LogicError); + addPostEvent(s, Multiplexor::ErrorEvent); + logWarn("h %d s %s %d unwatch fail", + id(), s->peer(), s->fd(), s->db()); + } } break; case Command::DiscardServ: if (!res->isOk()) { - s->setStatus(ConnectConnection::LogicError); - addPostEvent(s, Multiplexor::ErrorEvent); - logWarn("h %d s %s %d discard fail", - id(), s->peer(), s->fd(), s->db()); + if (s) { + s->setStatus(ConnectConnection::LogicError); + addPostEvent(s, Multiplexor::ErrorEvent); + logWarn("h %d s %s %d discard fail", + id(), s->peer(), s->fd(), s->db()); + } } break; default: @@ -1574,7 +1586,7 @@ bool Handler::redirect(ConnectConnection* c, Request* req, Response* res, bool m } } auto p = static_cast(mProxy->serverPool()); - Server* serv = p->redirect(addr, c->server()); + Server* serv = p->redirect(addr, c ? c->server() : nullptr); if (!serv) { logDebug("h %d req %ld %s redirect to %s can't get server", id(), req->id(), (moveOrAsk ? "MOVE" : "ASK"), addr.data()); diff --git a/src/RequestParser.cpp b/src/RequestParser.cpp index 7aa6573..06566b7 100644 --- a/src/RequestParser.cpp +++ b/src/RequestParser.cpp @@ -109,7 +109,9 @@ RequestParser::Status RequestParser::parse(Buffer* buf, int& pos, bool split) break; case InlineCmd: if (isspace(ch)) { - mCmd[mArgLen < Const::MaxCmdLen ? mArgLen : Const::MaxCmdLen - 1] = '\0'; + // Ensure we don't write out of bounds + int idx = (mArgLen >= 0 && mArgLen < Const::MaxCmdLen) ? mArgLen : Const::MaxCmdLen - 1; + mCmd[idx] = '\0'; parseCmd(); mArgCnt = 1; if (ch == '\n') { @@ -119,7 +121,8 @@ RequestParser::Status RequestParser::parse(Buffer* buf, int& pos, bool split) } mState = InlineArgBegin; } else { - if (mArgLen < Const::MaxCmdLen) { + // Only write if within bounds + if (mArgLen >= 0 && mArgLen < Const::MaxCmdLen) { mCmd[mArgLen] = tolower(ch); } ++mArgLen; @@ -238,7 +241,8 @@ RequestParser::Status RequestParser::parse(Buffer* buf, int& pos, bool split) mCmd[Const::MaxCmdLen - 1] = '\0'; ch == '\r' ? mState = CmdBodyLF : error = __LINE__; } else { - if (mArgBodyCnt < Const::MaxCmdLen) { + // Only write if within bounds + if (mArgBodyCnt >= 0 && mArgBodyCnt < Const::MaxCmdLen) { mCmd[mArgBodyCnt] = ch; } ++mArgBodyCnt; diff --git a/src/SentinelServerPool.cpp b/src/SentinelServerPool.cpp index 211b7fb..b8b288d 100644 --- a/src/SentinelServerPool.cpp +++ b/src/SentinelServerPool.cpp @@ -8,6 +8,7 @@ #include "Logger.h" #include "ServerGroup.h" #include "Handler.h" +#include "Conf.h" #include "SentinelServerPool.h" SentinelServerPool::SentinelServerPool(Proxy* p): diff --git a/src/ServerPool.h b/src/ServerPool.h index c41fe17..09d9544 100644 --- a/src/ServerPool.h +++ b/src/ServerPool.h @@ -20,7 +20,8 @@ public: { Unknown, Cluster, - Standalone + Standalone, + Sentinel }; static const int DefaultServerRetryTimeout = 10000000; static const int DefaultRefreshInterval = 1000000;