From 28ea9b9f601d9a56e3d1c4e90d83418e5ea58082 Mon Sep 17 00:00:00 2001 From: xiao-77 Date: Tue, 10 Dec 2024 20:13:53 +0800 Subject: [PATCH] Fix review errors. --- source/common/src/tglobal.c | 15 ++++----- source/dnode/mgmt/mgmt_dnode/src/dmHandle.c | 11 ++++++- source/dnode/mgmt/mgmt_dnode/src/dmWorker.c | 1 - source/dnode/mnode/impl/src/mndConfig.c | 24 ++++++++++++--- source/util/src/tconfig.c | 34 +++++++++++++-------- 5 files changed, 58 insertions(+), 27 deletions(-) diff --git a/source/common/src/tglobal.c b/source/common/src/tglobal.c index 8defbbc546..d6add7f3d9 100644 --- a/source/common/src/tglobal.c +++ b/source/common/src/tglobal.c @@ -1425,9 +1425,6 @@ static int32_t taosSetSystemCfg(SConfig *pCfg) { tsEnableCoreFile = pItem->bval; taosSetCoreDump(tsEnableCoreFile); - TAOS_CHECK_GET_CFG_ITEM(pCfg, pItem, "assert"); - tsAssert = pItem->bval; - // todo tsVersion = 30000000; @@ -1981,7 +1978,11 @@ int32_t cfgDeserialize(SArray *array, char *buf, bool isGlobal) { int64_t actDiskID = 0; int64_t expDiskID = taosStr2Int64(cJSON_GetStringValue(filed), NULL, 10); - if (!taosCheckFileDiskID(dir, &actDiskID, expDiskID)) { + if ((code = taosGetFileDiskID(dir, &actDiskID)) != 0) { + uError("failed to get disk id for dir:%s, since %s", dir, tstrerror(code)); + goto _exit; + } + if (actDiskID != expDiskID) { uError("failed to check disk id for dir:%s, actDiskID%" PRId64 ", expDiskID%" PRId64, dir, actDiskID, expDiskID); code = TSDB_CODE_INVALID_DISK_ID; @@ -2006,7 +2007,7 @@ int32_t cfgDeserialize(SArray *array, char *buf, bool isGlobal) { break; case CFG_DTYPE_FLOAT: case CFG_DTYPE_DOUBLE: - pItem->fval = atoll(cJSON_GetStringValue(pJson)); + pItem->fval = taosStr2Float(cJSON_GetStringValue(pJson), NULL); break; case CFG_DTYPE_STRING: case CFG_DTYPE_DIR: @@ -2959,7 +2960,7 @@ int32_t taosPersistLocalConfig(const char *path) { taosOpenFile(filename, TD_FILE_CREATE | TD_FILE_WRITE | TD_FILE_TRUNC | TD_FILE_WRITE_THROUGH); if (pConfigFile == NULL) { - code = TAOS_SYSTEM_ERROR(errno); + code = TAOS_SYSTEM_ERROR(terrno); uError("failed to open file:%s since %s", filename, tstrerror(code)); TAOS_RETURN(code); } @@ -2968,7 +2969,7 @@ int32_t taosPersistLocalConfig(const char *path) { TAOS_CHECK_GOTO(localConfigSerialize(taosGetLocalCfg(tsCfg), &serialized), &lino, _exit); if (taosWriteFile(pConfigFile, serialized, strlen(serialized)) < 0) { lino = __LINE__; - code = TAOS_SYSTEM_ERROR(errno); + code = TAOS_SYSTEM_ERROR(terrno); uError("failed to write file:%s since %s", filename, tstrerror(code)); goto _exit; } diff --git a/source/dnode/mgmt/mgmt_dnode/src/dmHandle.c b/source/dnode/mgmt/mgmt_dnode/src/dmHandle.c index 906cc30946..f9a0ef8925 100644 --- a/source/dnode/mgmt/mgmt_dnode/src/dmHandle.c +++ b/source/dnode/mgmt/mgmt_dnode/src/dmHandle.c @@ -198,7 +198,8 @@ void dmSendStatusReq(SDnodeMgmt *pMgmt) { req.clusterCfg.monitorParas.tsSlowLogThreshold = tsSlowLogThreshold; tstrncpy(req.clusterCfg.monitorParas.tsSlowLogExceptDb, tsSlowLogExceptDb, TSDB_DB_NAME_LEN); char timestr[32] = "1970-01-01 00:00:00.00"; - if (taosParseTime(timestr, &req.clusterCfg.checkTime, (int32_t)strlen(timestr), TSDB_TIME_PRECISION_MILLI, NULL) != 0) { + if (taosParseTime(timestr, &req.clusterCfg.checkTime, (int32_t)strlen(timestr), TSDB_TIME_PRECISION_MILLI, NULL) != + 0) { dError("failed to parse time since %s", tstrerror(code)); } memcpy(req.clusterCfg.timezone, tsTimezoneStr, TD_TIMEZONE_LEN); @@ -333,6 +334,10 @@ static void dmProcessConfigRsp(SDnodeMgmt *pMgmt, SRpcMsg *pRsp) { } if (needUpdate) { code = cfgUpdateFromArray(tsCfg, configRsp.array); + if (code != TSDB_CODE_SUCCESS) { + dError("failed to update config since %s", tstrerror(code)); + goto _exit; + } code = setAllConfigs(tsCfg); if (code != TSDB_CODE_SUCCESS) { dError("failed to set all configs since %s", tstrerror(code)); @@ -369,6 +374,10 @@ void dmSendConfigReq(SDnodeMgmt *pMgmt) { } void *pHead = rpcMallocCont(contLen); + if (pHead == NULL) { + dError("failed to malloc cont since %s", tstrerror(contLen)); + return; + } contLen = tSerializeSConfigReq(pHead, contLen, &req); if (contLen < 0) { rpcFreeCont(pHead); diff --git a/source/dnode/mgmt/mgmt_dnode/src/dmWorker.c b/source/dnode/mgmt/mgmt_dnode/src/dmWorker.c index b481a3fc5e..fb810ef0c4 100644 --- a/source/dnode/mgmt/mgmt_dnode/src/dmWorker.c +++ b/source/dnode/mgmt/mgmt_dnode/src/dmWorker.c @@ -51,7 +51,6 @@ static void *dmConfigThreadFp(void *param) { SDnodeMgmt *pMgmt = param; int64_t lastTime = taosGetTimestampMs(); setThreadName("dnode-config"); - int32_t upTimeCount = 0; while (1) { taosMsleep(200); if (pMgmt->pData->dropped || pMgmt->pData->stopped || tsConfigInited) break; diff --git a/source/dnode/mnode/impl/src/mndConfig.c b/source/dnode/mnode/impl/src/mndConfig.c index 1155495fd4..0143a5d367 100644 --- a/source/dnode/mnode/impl/src/mndConfig.c +++ b/source/dnode/mnode/impl/src/mndConfig.c @@ -244,6 +244,10 @@ static int32_t mndProcessConfigReq(SRpcMsg *pReq) { } array = taosArrayInit(16, sizeof(SConfigItem)); + if (array == NULL) { + code = TSDB_CODE_OUT_OF_MEMORY; + goto _OVER; + } SConfigRsp configRsp = {0}; configRsp.forceReadConfig = configReq.forceReadConfig; @@ -291,6 +295,7 @@ _OVER: if (code != 0) { mError("failed to process config req, since %s", tstrerror(code)); } + sdbRelease(pMnode->pSdb, vObj); cfgArrayCleanUp(array); mndReleaseDnode(pMnode, pDnode); return TSDB_CODE_SUCCESS; @@ -310,7 +315,7 @@ int32_t mndInitWriteCfg(SMnode *pMnode) { // encode mnd config version SConfigObj *versionObj = mndInitConfigVersion(); if ((code = mndSetCreateConfigCommitLogs(pTrans, versionObj)) != 0) { - mError("failed to init mnd config version, since %s", terrstr()); + mError("failed to init mnd config version, since %s", tstrerror(code)); taosMemoryFree(versionObj->str); taosMemoryFree(versionObj); goto _OVER; @@ -327,6 +332,8 @@ int32_t mndInitWriteCfg(SMnode *pMnode) { } if ((code = mndSetCreateConfigCommitLogs(pTrans, obj)) != 0) { mError("failed to init mnd config:%s, since %s", item->name, tstrerror(code)); + taosMemoryFree(obj); + goto _OVER; } taosMemoryFree(obj); } @@ -347,7 +354,7 @@ int32_t mndInitReadCfg(SMnode *pMnode) { if (obj == NULL) { code = mndInitWriteCfg(pMnode); if (code != 0) { - mError("failed to init write cfg, since %s", terrstr()); + mError("failed to init write cfg, since %s", tstrerror(code)); } mInfo("failed to acquire mnd config version, try to rebuild it , since %s", terrstr()); goto _OVER; @@ -426,17 +433,17 @@ static int32_t mndMCfg2DCfg(SMCfgDnodeReq *pMCfgReq, SDCfgDnodeReq *pDCfgReq) { } size_t optLen = p - pMCfgReq->config; - tstrncpy(pDCfgReq->config, pMCfgReq->config, optLen + 1); + strncpy(pDCfgReq->config, pMCfgReq->config, optLen); pDCfgReq->config[optLen] = 0; if (' ' == pMCfgReq->config[optLen]) { // 'key value' if (strlen(pMCfgReq->value) != 0) goto _err; - tstrncpy(pDCfgReq->value, p + 1, strlen(p + 1)); + (void)strcpy(pDCfgReq->value, p + 1); } else { // 'key' 'value' if (strlen(pMCfgReq->value) == 0) goto _err; - tstrncpy(pDCfgReq->value, pMCfgReq->value, strlen(pMCfgReq->value)); + (void)strcpy(pDCfgReq->value, pMCfgReq->value); } TAOS_RETURN(code); @@ -659,6 +666,7 @@ static int32_t initConfigArrayFromSdb(SMnode *pMnode, SArray *array) { if (item.name == NULL) { code = terrno; sdbCancelFetch(pSdb, pIter); + sdbRelease(pSdb, obj); goto _exit; } switch (obj->dtype) { @@ -685,6 +693,7 @@ static int32_t initConfigArrayFromSdb(SMnode *pMnode, SArray *array) { item.str = taosStrdup(obj->str); if (item.str == NULL) { sdbCancelFetch(pSdb, pIter); + sdbRelease(pSdb, obj); code = terrno; goto _exit; } @@ -692,6 +701,7 @@ static int32_t initConfigArrayFromSdb(SMnode *pMnode, SArray *array) { } if (taosArrayPush(array, &item) == NULL) { sdbCancelFetch(pSdb, pIter); + sdbRelease(pSdb, obj); code = TSDB_CODE_OUT_OF_MEMORY; goto _exit; break; @@ -731,6 +741,10 @@ SArray *initVariablesFromItems(SArray *pItems) { int32_t sz = taosArrayGetSize(pItems); SArray *pInfos = taosArrayInit(sz, sizeof(SVariablesInfo)); + if (pInfos == NULL) { + mError("failed to init array while init variables from items, since %s", tstrerror(terrno)); + return NULL; + } for (int32_t i = 0; i < sz; ++i) { SConfigItem *pItem = taosArrayGet(pItems, i); SVariablesInfo info = {0}; diff --git a/source/util/src/tconfig.c b/source/util/src/tconfig.c index a6b2fc6a05..f18fdfe970 100644 --- a/source/util/src/tconfig.c +++ b/source/util/src/tconfig.c @@ -17,6 +17,7 @@ #include "cJSON.h" #include "taoserror.h" #include "tconfig.h" +#include "tconv.h" #include "tenv.h" #include "tglobal.h" #include "tgrant.h" @@ -24,7 +25,6 @@ #include "tlog.h" #include "tunit.h" #include "tutil.h" -#include "tconv.h" #define CFG_NAME_PRINT_LEN 32 #define CFG_SRC_PRINT_LEN 12 @@ -129,6 +129,10 @@ int32_t cfgUpdateFromArray(SConfig *pCfg, SArray *pArgs) { case CFG_DTYPE_TIMEZONE: taosMemoryFree(pItemOld->str); pItemOld->str = taosStrdup(pItemNew->str); + if (pItemOld->str == NULL) { + (void)taosThreadMutexUnlock(&pCfg->lock); + TAOS_RETURN(terrno); + } break; default: break; @@ -307,7 +311,7 @@ static int32_t doSetConf(SConfigItem *pItem, const char *value, ECfgSrcType styp } static int32_t cfgSetTimezone(SConfigItem *pItem, const char *value, ECfgSrcType stype) { - if (stype == CFG_STYPE_ALTER_SERVER_CMD || (pItem->dynScope & CFG_DYN_CLIENT) == 0){ + if (stype == CFG_STYPE_ALTER_SERVER_CMD || (pItem->dynScope & CFG_DYN_CLIENT) == 0) { uError("failed to config timezone, not support"); TAOS_RETURN(TSDB_CODE_INVALID_CFG); } @@ -325,7 +329,7 @@ static int32_t cfgSetTimezone(SConfigItem *pItem, const char *value, ECfgSrcType } static int32_t cfgSetCharset(SConfigItem *pItem, const char *value, ECfgSrcType stype) { - if (stype == CFG_STYPE_ALTER_SERVER_CMD || stype == CFG_STYPE_ALTER_CLIENT_CMD){ + if (stype == CFG_STYPE_ALTER_SERVER_CMD || stype == CFG_STYPE_ALTER_CLIENT_CMD) { uError("failed to config charset, not support"); TAOS_RETURN(TSDB_CODE_INVALID_CFG); } @@ -351,7 +355,7 @@ static int32_t cfgSetCharset(SConfigItem *pItem, const char *value, ECfgSrcType } static int32_t cfgSetLocale(SConfigItem *pItem, const char *value, ECfgSrcType stype) { - if (stype == CFG_STYPE_ALTER_SERVER_CMD || (pItem->dynScope & CFG_DYN_CLIENT) == 0){ + if (stype == CFG_STYPE_ALTER_SERVER_CMD || (pItem->dynScope & CFG_DYN_CLIENT) == 0) { uError("failed to config locale, not support"); TAOS_RETURN(TSDB_CODE_INVALID_CFG); } @@ -589,7 +593,11 @@ int32_t cfgCheckRangeForDynUpdate(SConfig *pCfg, const char *name, const char *p cfgUnLock(pCfg); TAOS_RETURN(TSDB_CODE_CFG_NOT_FOUND); } - TAOS_CHECK_RETURN(checkItemDyn(pItem, isServer)); + int32_t code = checkItemDyn(pItem, isServer); + if (code != TSDB_CODE_SUCCESS) { + cfgUnLock(pCfg); + TAOS_RETURN(code); + } if ((!isUpdateAll) && (pItem->category == CFG_CATEGORY_GLOBAL)) { uError("failed to config:%s, not support update global config on only one dnode", name); cfgUnLock(pCfg); @@ -872,14 +880,14 @@ int32_t cfgDumpItemValue(SConfigItem *pItem, char *buf, int32_t bufSize, int32_t case CFG_DTYPE_DOUBLE: len = tsnprintf(buf, bufSize, "%f", pItem->fval); break; - case CFG_DTYPE_TIMEZONE:{ -// char str1[TD_TIMEZONE_LEN] = {0}; -// time_t tx1 = taosGetTimestampSec(); -// if (taosFormatTimezoneStr(tx1, buf, NULL, str1) != 0) { -// tstrncpy(str1, "tz error", sizeof(str1)); -// } -// len = tsnprintf(buf, bufSize, "%s", str1); -// break; + case CFG_DTYPE_TIMEZONE: { + // char str1[TD_TIMEZONE_LEN] = {0}; + // time_t tx1 = taosGetTimestampSec(); + // if (taosFormatTimezoneStr(tx1, buf, NULL, str1) != 0) { + // tstrncpy(str1, "tz error", sizeof(str1)); + // } + // len = tsnprintf(buf, bufSize, "%s", str1); + // break; } case CFG_DTYPE_STRING: case CFG_DTYPE_DIR: