Apply suggestions from code review

This commit is contained in:
Zwelf 2020-07-08 21:31:34 +02:00
parent b898f8c7c2
commit 2c5f892205
4 changed files with 27 additions and 23 deletions

View file

@ -435,7 +435,7 @@ if(NOT(PYTHONINTERP_FOUND))
message(SEND_ERROR "You must install Python to compile DDNet") message(SEND_ERROR "You must install Python to compile DDNet")
endif() endif()
if(NOT(SQLite3_FOUND)) if(NOT(SQLite3_FOUND))
message(SEND_ERROR "You must install SQLite3 to compile the DDNet server") message(SEND_ERROR "You must install SQLite3 to compile DDNet")
endif() endif()
if(MYSQL AND NOT(MYSQL_FOUND)) if(MYSQL AND NOT(MYSQL_FOUND))

View file

@ -9,11 +9,11 @@
struct CSqlExecData struct CSqlExecData
{ {
CSqlExecData( CSqlExecData(
bool (*pFuncPtr) (IDbConnection *, const ISqlData *), CDbConnectionPool::FRead pFunc,
std::unique_ptr<const ISqlData> pThreadData, std::unique_ptr<const ISqlData> pThreadData,
const char *pName); const char *pName);
CSqlExecData( CSqlExecData(
bool (*pFuncPtr) (IDbConnection *, const ISqlData *, bool), CDbConnectionPool::FWrite pFunc,
std::unique_ptr<const ISqlData> pThreadData, std::unique_ptr<const ISqlData> pThreadData,
const char *pName); const char *pName);
~CSqlExecData() {} ~CSqlExecData() {}
@ -25,8 +25,8 @@ struct CSqlExecData
} m_Mode; } m_Mode;
union union
{ {
bool (*m_pWriteFunc) (IDbConnection*, const ISqlData *, bool); CDbConnectionPool::FRead m_pReadFunc;
bool (*m_pReadFunc) (IDbConnection*, const ISqlData *); CDbConnectionPool::FWrite m_pWriteFunc;
} m_Ptr; } m_Ptr;
std::unique_ptr<const ISqlData> m_pThreadData; std::unique_ptr<const ISqlData> m_pThreadData;
@ -34,25 +34,25 @@ struct CSqlExecData
}; };
CSqlExecData::CSqlExecData( CSqlExecData::CSqlExecData(
bool (*pFuncPtr) (IDbConnection *, const ISqlData *), CDbConnectionPool::FRead pFunc,
std::unique_ptr<const ISqlData> pThreadData, std::unique_ptr<const ISqlData> pThreadData,
const char *pName) : const char *pName) :
m_Mode(READ_ACCESS), m_Mode(READ_ACCESS),
m_pThreadData(std::move(pThreadData)), m_pThreadData(std::move(pThreadData)),
m_pName(pName) m_pName(pName)
{ {
m_Ptr.m_pReadFunc = pFuncPtr; m_Ptr.m_pReadFunc = pFunc;
} }
CSqlExecData::CSqlExecData( CSqlExecData::CSqlExecData(
bool (*pFuncPtr) (IDbConnection *, const ISqlData *, bool), CDbConnectionPool::FWrite pFunc,
std::unique_ptr<const ISqlData> pThreadData, std::unique_ptr<const ISqlData> pThreadData,
const char *pName) : const char *pName) :
m_Mode(WRITE_ACCESS), m_Mode(WRITE_ACCESS),
m_pThreadData(std::move(pThreadData)), m_pThreadData(std::move(pThreadData)),
m_pName(pName) m_pName(pName)
{ {
m_Ptr.m_pWriteFunc = pFuncPtr; m_Ptr.m_pWriteFunc = pFunc;
} }
CDbConnectionPool::CDbConnectionPool() : CDbConnectionPool::CDbConnectionPool() :
@ -60,7 +60,7 @@ CDbConnectionPool::CDbConnectionPool() :
FirstElem(0), FirstElem(0),
LastElem(0) LastElem(0)
{ {
thread_init_and_detach(CDbConnectionPool::SqlWorker, this, "database worker thread"); thread_init_and_detach(CDbConnectionPool::Worker, this, "database worker thread");
} }
CDbConnectionPool::~CDbConnectionPool() CDbConnectionPool::~CDbConnectionPool()
@ -75,21 +75,21 @@ void CDbConnectionPool::RegisterDatabase(std::unique_ptr<IDbConnection> pDatabas
} }
void CDbConnectionPool::Execute( void CDbConnectionPool::Execute(
bool (*pFuncPtr) (IDbConnection *, const ISqlData *), FRead pFunc,
std::unique_ptr<const ISqlData> pThreadData, std::unique_ptr<const ISqlData> pThreadData,
const char *pName) const char *pName)
{ {
m_aTasks[FirstElem++].reset(new CSqlExecData(pFuncPtr, std::move(pThreadData), pName)); m_aTasks[FirstElem++].reset(new CSqlExecData(pFunc, std::move(pThreadData), pName));
FirstElem %= sizeof(m_aTasks) / sizeof(m_aTasks[0]); FirstElem %= sizeof(m_aTasks) / sizeof(m_aTasks[0]);
m_NumElem.signal(); m_NumElem.signal();
} }
void CDbConnectionPool::ExecuteWrite( void CDbConnectionPool::ExecuteWrite(
bool (*pFuncPtr) (IDbConnection *, const ISqlData *, bool), FWrite pFunc,
std::unique_ptr<const ISqlData> pThreadData, std::unique_ptr<const ISqlData> pThreadData,
const char *pName) const char *pName)
{ {
m_aTasks[FirstElem++].reset(new CSqlExecData(pFuncPtr, std::move(pThreadData), pName)); m_aTasks[FirstElem++].reset(new CSqlExecData(pFunc, std::move(pThreadData), pName));
FirstElem %= sizeof(m_aTasks) / sizeof(m_aTasks[0]); FirstElem %= sizeof(m_aTasks) / sizeof(m_aTasks[0]);
m_NumElem.signal(); m_NumElem.signal();
} }
@ -98,13 +98,13 @@ void CDbConnectionPool::OnShutdown()
{ {
} }
void CDbConnectionPool::SqlWorker(void *pUser) void CDbConnectionPool::Worker(void *pUser)
{ {
CDbConnectionPool *pThis = (CDbConnectionPool *)pUser; CDbConnectionPool *pThis = (CDbConnectionPool *)pUser;
pThis->SqlWorker(); pThis->Worker();
} }
void CDbConnectionPool::SqlWorker() void CDbConnectionPool::Worker()
{ {
while(1) while(1)
{ {
@ -158,7 +158,8 @@ bool CDbConnectionPool::ExecSqlFunc(IDbConnection *pConnection, CSqlExecData *pD
if(pConnection->Connect() != IDbConnection::SUCCESS) if(pConnection->Connect() != IDbConnection::SUCCESS)
return false; return false;
bool Success = false; bool Success = false;
try { try
{
switch(pData->m_Mode) switch(pData->m_Mode)
{ {
case CSqlExecData::READ_ACCESS: case CSqlExecData::READ_ACCESS:

View file

@ -20,6 +20,9 @@ public:
~CDbConnectionPool(); ~CDbConnectionPool();
CDbConnectionPool& operator=(const CDbConnectionPool&) = delete; CDbConnectionPool& operator=(const CDbConnectionPool&) = delete;
typedef bool (*FRead)(IDbConnection *, const ISqlData *);
typedef bool (*FWrite)(IDbConnection *, const ISqlData *, bool);
enum Mode enum Mode
{ {
READ, READ,
@ -31,12 +34,12 @@ public:
void RegisterDatabase(std::unique_ptr<IDbConnection> pDatabase, Mode DatabaseMode); void RegisterDatabase(std::unique_ptr<IDbConnection> pDatabase, Mode DatabaseMode);
void Execute( void Execute(
bool (*pFuncPtr) (IDbConnection *, const ISqlData *), FRead pFunc,
std::unique_ptr<const ISqlData> pSqlRequestData, std::unique_ptr<const ISqlData> pSqlRequestData,
const char *pName); const char *pName);
// writes to WRITE_BACKUP server in case of failure // writes to WRITE_BACKUP server in case of failure
void ExecuteWrite( void ExecuteWrite(
bool (*pFuncPtr) (IDbConnection *, const ISqlData *, bool), FWrite pFunc,
std::unique_ptr<const ISqlData> pSqlRequestData, std::unique_ptr<const ISqlData> pSqlRequestData,
const char *pName); const char *pName);
@ -45,8 +48,8 @@ public:
private: private:
std::vector<std::unique_ptr<IDbConnection>> m_aapDbConnections[NUM_MODES]; std::vector<std::unique_ptr<IDbConnection>> m_aapDbConnections[NUM_MODES];
static void SqlWorker(void *pUser); static void Worker(void *pUser);
void SqlWorker(); void Worker();
bool ExecSqlFunc(IDbConnection *pConnection, struct CSqlExecData *pData, bool Failure); bool ExecSqlFunc(IDbConnection *pConnection, struct CSqlExecData *pData, bool Failure);
semaphore m_NumElem; semaphore m_NumElem;

View file

@ -219,7 +219,7 @@ MACRO_CONFIG_INT(SvSaveGames, sv_savegames, 1, 0, 1, CFGFLAG_SERVER, "Enables sa
MACRO_CONFIG_INT(SvSaveGamesDelay, sv_savegames_delay, 60, 0, 10000, CFGFLAG_SERVER, "Delay in seconds for loading a savegame") MACRO_CONFIG_INT(SvSaveGamesDelay, sv_savegames_delay, 60, 0, 10000, CFGFLAG_SERVER, "Delay in seconds for loading a savegame")
MACRO_CONFIG_INT(SvUseSQL, sv_use_sql, 0, 0, 1, CFGFLAG_SERVER, "Enables SQL DB instead of record file") MACRO_CONFIG_INT(SvUseSQL, sv_use_sql, 0, 0, 1, CFGFLAG_SERVER, "Enables SQL DB instead of record file")
MACRO_CONFIG_INT(SvSqlQueriesDelay, sv_sql_queries_delay, 1, 0, 20, CFGFLAG_SERVER, "Delay in seconds between SQL queries of a single player") MACRO_CONFIG_INT(SvSqlQueriesDelay, sv_sql_queries_delay, 1, 0, 20, CFGFLAG_SERVER, "Delay in seconds between SQL queries of a single player")
MACRO_CONFIG_STR(SvSqliteFile, sv_sqlite_file, 64, "ddnet.sqlite", CFGFLAG_SERVER, "File to store ranks in case sv_use_sql is turned off or used as backup sql server") MACRO_CONFIG_STR(SvSqliteFile, sv_sqlite_file, 64, "ddnet-server.sqlite", CFGFLAG_SERVER, "File to store ranks in case sv_use_sql is turned off or used as backup sql server")
#if defined(CONF_UPNP) #if defined(CONF_UPNP)
MACRO_CONFIG_INT(SvUseUPnP, sv_use_upnp, 0, 0, 1, CFGFLAG_SERVER, "Enables UPnP support.") MACRO_CONFIG_INT(SvUseUPnP, sv_use_upnp, 0, 0, 1, CFGFLAG_SERVER, "Enables UPnP support.")