From 371a8eec29fa25bbf58f4b1f0d7e3bf4c3ad6329 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 16 Dec 2010 12:57:55 +0100 Subject: some cleanup based on clang static analyzer results --- ChangeLog | 4 ++++ plugins/imklog/ksym.c | 4 +--- runtime/cfsysline.c | 4 ++-- runtime/debug.c | 6 ++---- runtime/msg.c | 4 +--- runtime/parser.c | 2 -- runtime/queue.c | 5 ++--- runtime/wtp.c | 2 +- template.c | 10 ++++------ threads.c | 3 +-- tools/omfile.c | 1 - tools/omfwd.c | 2 -- tools/omusrmsg.c | 1 - tools/syslogd.c | 2 -- 14 files changed, 18 insertions(+), 32 deletions(-) diff --git a/ChangeLog b/ChangeLog index 731939d8..3c7d3c4c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,6 +5,10 @@ Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? - bugfix: imfile utilizes 32 bit to track offset. Most importantly, this problem can not experienced on Fedora 64 bit OS (which has 64 bit long's!) +- some improvements thanks to clang's static code analyzer + o overall cleanup (mostly unnecessary writes and otherwise unused stuff) + o bugfix: fixed a very remote problem in msg.c which could occur when + running under extremely low memory conditions --------------------------------------------------------------------------- Version 4.6.5 [v4-stable] (rgerhards), 2010-11-24 - bugfix(important): problem in TLS handling could cause rsyslog to loop diff --git a/plugins/imklog/ksym.c b/plugins/imklog/ksym.c index f636a7bb..ca708ba6 100644 --- a/plugins/imklog/ksym.c +++ b/plugins/imklog/ksym.c @@ -650,8 +650,7 @@ static void FreeSymbols(void) **************************************************************************/ extern char *ExpandKadds(char *line, char *el) { - auto char dlm, - *kp, + auto char *kp, *sl = line, *elp = el, *symbol; @@ -781,7 +780,6 @@ extern char *ExpandKadds(char *line, char *el) strcpy(el, sl); return(el); } - dlm = *kp; strncpy(num,sl+1,kp-sl-1); num[kp-sl-1] = '\0'; value = strtoul(num, (char **) 0, 16); diff --git a/runtime/cfsysline.c b/runtime/cfsysline.c index 469fbfc2..1dd5905e 100644 --- a/runtime/cfsysline.c +++ b/runtime/cfsysline.c @@ -961,11 +961,11 @@ void dbgPrintCfSysLineHandlers(void) dbgprintf("Sytem Line Configuration Commands:\n"); llCookieCmd = NULL; - while((iRet = llGetNextElt(&llCmdList, &llCookieCmd, (void*)&pCmd)) == RS_RET_OK) { + while(llGetNextElt(&llCmdList, &llCookieCmd, (void*)&pCmd) == RS_RET_OK) { llGetKey(llCookieCmd, (void*) &pKey); /* TODO: using the cookie is NOT clean! */ dbgprintf("\tCommand '%s':\n", pKey); llCookieCmdHdlr = NULL; - while((iRet = llGetNextElt(&pCmd->llCmdHdlrs, &llCookieCmdHdlr, (void*)&pCmdHdlr)) == RS_RET_OK) { + while(llGetNextElt(&pCmd->llCmdHdlrs, &llCookieCmdHdlr, (void*)&pCmdHdlr) == RS_RET_OK) { dbgprintf("\t\ttype : %d\n", pCmdHdlr->eType); dbgprintf("\t\tpData: 0x%lx\n", (unsigned long) pCmdHdlr->pData); dbgprintf("\t\tHdlr : 0x%lx\n", (unsigned long) pCmdHdlr->cslCmdHdlr); diff --git a/runtime/debug.c b/runtime/debug.c index 9b7c2952..6bb8d743 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -433,14 +433,13 @@ dbgMutLog_t *dbgMutLogFindHolder(pthread_mutex_t *pmut) static inline void dbgMutexPreLockLog(pthread_mutex_t *pmut, dbgFuncDB_t *pFuncDB, int ln) { dbgMutLog_t *pHolder; - dbgMutLog_t *pLog; char pszBuf[128]; char pszHolderThrdName[64]; char *pszHolder; pthread_mutex_lock(&mutMutLog); pHolder = dbgMutLogFindHolder(pmut); - pLog = dbgMutLogAddEntry(pmut, MUTOP_LOCKWAIT, pFuncDB, ln); + dbgMutLogAddEntry(pmut, MUTOP_LOCKWAIT, pFuncDB, ln); if(pHolder == NULL) pszHolder = "[NONE]"; @@ -481,14 +480,13 @@ static inline void dbgMutexLockLog(pthread_mutex_t *pmut, dbgFuncDB_t *pFuncDB, static inline void dbgMutexPreTryLockLog(pthread_mutex_t *pmut, dbgFuncDB_t *pFuncDB, int ln) { dbgMutLog_t *pHolder; - dbgMutLog_t *pLog; char pszBuf[128]; char pszHolderThrdName[64]; char *pszHolder; pthread_mutex_lock(&mutMutLog); pHolder = dbgMutLogFindHolder(pmut); - pLog = dbgMutLogAddEntry(pmut, MUTOP_TRYLOCK, pFuncDB, ln); + dbgMutLogAddEntry(pmut, MUTOP_TRYLOCK, pFuncDB, ln); if(pHolder == NULL) pszHolder = "[NONE]"; diff --git a/runtime/msg.c b/runtime/msg.c index f5041b85..a9a09143 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -1476,7 +1476,7 @@ rsRetVal MsgSetPROCID(msg_t *pMsg, char* pszPROCID) CHKiRet(cstrConstruct(&pMsg->pCSPROCID)); } /* if we reach this point, we have the object */ - iRet = rsCStrSetSzStr(pMsg->pCSPROCID, (uchar*) pszPROCID); + CHKiRet(rsCStrSetSzStr(pMsg->pCSPROCID, (uchar*) pszPROCID)); CHKiRet(cstrFinalize(pMsg->pCSPROCID)); finalize_it: @@ -2872,7 +2872,6 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pTpe->data.field.options.bCSV) { /* we need to obtain a private copy, as we need to at least add the double quotes */ int iBufLen; - int i; uchar *pBStart; uchar *pDst; uchar *pSrc; @@ -2887,7 +2886,6 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, RET_OUT_OF_MEMORY; } pSrc = pRes; - i = 0; *pDst++ = '"'; /* starting quote */ while(*pSrc) { if(*pSrc == '"') diff --git a/runtime/parser.c b/runtime/parser.c index 36e88ebd..d27f3e38 100644 --- a/runtime/parser.c +++ b/runtime/parser.c @@ -272,7 +272,6 @@ rsRetVal parseMsg(msg_t *pMsg) uchar *msg; int pri; int lenMsg; - int iPriText; if(pMsg->iLenRawMsg == 0) ABORT_FINALIZE(RS_RET_EMPTY_MSG); @@ -286,7 +285,6 @@ rsRetVal parseMsg(msg_t *pMsg) lenMsg = pMsg->iLenRawMsg; msg = pMsg->pszRawMsg; pri = DEFUPRI; - iPriText = 0; if(*msg == '<') { /* while we process the PRI, we also fill the PRI textual representation * inside the msg object. This may not be ideal from an OOP point of view, diff --git a/runtime/queue.c b/runtime/queue.c index 9d7a9058..39898718 100644 --- a/runtime/queue.c +++ b/runtime/queue.c @@ -685,7 +685,6 @@ qqueueHaveQIF(qqueue_t *pThis) { DEFiRet; uchar pszQIFNam[MAXFNAME]; - size_t lenQIFNam; struct stat stat_buf; ISOBJ_TYPE_assert(pThis, qqueue); @@ -694,8 +693,8 @@ qqueueHaveQIF(qqueue_t *pThis) ABORT_FINALIZE(RS_RET_NO_FILEPREFIX); /* Construct file name */ - lenQIFNam = snprintf((char*)pszQIFNam, sizeof(pszQIFNam) / sizeof(uchar), "%s/%s.qi", - (char*) glbl.GetWorkDir(), (char*)pThis->pszFilePrefix); + snprintf((char*)pszQIFNam, sizeof(pszQIFNam) / sizeof(uchar), "%s/%s.qi", + (char*) glbl.GetWorkDir(), (char*)pThis->pszFilePrefix); /* check if the file exists */ if(stat((char*) pszQIFNam, &stat_buf) == -1) { diff --git a/runtime/wtp.c b/runtime/wtp.c index 0c66dd11..f71d5855 100644 --- a/runtime/wtp.c +++ b/runtime/wtp.c @@ -460,7 +460,7 @@ wtpWorker(void *arg) /* the arg is actually a wti object, even though we are in do { END_MTX_PROTECTED_OPERATIONS(&pThis->mut); - iRet = wtiWorker(pWti); /* just to make sure: this is NOT protected by the mutex! */ + wtiWorker(pWti); /* just to make sure: this is NOT protected by the mutex! */ BEGIN_MTX_PROTECTED_OPERATIONS(&pThis->mut, LOCK_MUTEX); } while(pThis->iCurNumWrkThrd == 1 && pThis->bInactivityGuard == 1); diff --git a/template.c b/template.c index 68c57be1..a5331d57 100644 --- a/template.c +++ b/template.c @@ -82,9 +82,9 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t * DEFiRet; struct templateEntry *pTpe; int iBuf; - unsigned short bMustBeFreed; + unsigned short bMustBeFreed = 0; uchar *pVal; - size_t iLenVal; + size_t iLenVal = 0; assert(pTpl != NULL); assert(pMsg != NULL); @@ -980,7 +980,6 @@ void tplDeleteAll(void) { struct template *pTpl, *pTplDel; struct templateEntry *pTpe, *pTpeDel; - rsRetVal iRetLocal; BEGINfunc pTpl = tplRoot; @@ -1003,7 +1002,7 @@ void tplDeleteAll(void) case FIELD: /* check if we have a regexp and, if so, delete it */ if(pTpeDel->data.field.has_regex != 0) { - if((iRetLocal = objUse(regexp, LM_REGEXP_FILENAME)) == RS_RET_OK) { + if(objUse(regexp, LM_REGEXP_FILENAME) == RS_RET_OK) { regexp.regfree(&(pTpeDel->data.field.re)); } } @@ -1029,7 +1028,6 @@ void tplDeleteNew(void) { struct template *pTpl, *pTplDel; struct templateEntry *pTpe, *pTpeDel; - rsRetVal iRetLocal; BEGINfunc @@ -1058,7 +1056,7 @@ void tplDeleteNew(void) case FIELD: /* check if we have a regexp and, if so, delete it */ if(pTpeDel->data.field.has_regex != 0) { - if((iRetLocal = objUse(regexp, LM_REGEXP_FILENAME)) == RS_RET_OK) { + if(objUse(regexp, LM_REGEXP_FILENAME) == RS_RET_OK) { regexp.regfree(&(pTpeDel->data.field.re)); } } diff --git a/threads.c b/threads.c index 13222694..051903de 100644 --- a/threads.c +++ b/threads.c @@ -151,7 +151,6 @@ rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), rsRetVal(*afterRun)(thrdI { DEFiRet; thrdInfo_t *pThis; - int i; assert(thrdMain != NULL); @@ -159,7 +158,7 @@ rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), rsRetVal(*afterRun)(thrdI pThis->bIsActive = 1; pThis->pUsrThrdMain = thrdMain; pThis->pAfterRun = afterRun; - i = pthread_create(&pThis->thrdID, NULL, thrdStarter, pThis); + pthread_create(&pThis->thrdID, NULL, thrdStarter, pThis); CHKiRet(llAppend(&llThrds, NULL, pThis)); finalize_it: diff --git a/tools/omfile.c b/tools/omfile.c index 24de052c..487cf8a0 100644 --- a/tools/omfile.c +++ b/tools/omfile.c @@ -353,7 +353,6 @@ prepareFile(instanceData *pData, uchar *newFileName) if(access((char*)newFileName, F_OK) != 0) { /* file does not exist, create it (and eventually parent directories */ - fd = -1; if(pData->bCreateDirs) { /* We first need to create parent dirs if they are missing. * We do not report any errors here ourselfs but let the code diff --git a/tools/omfwd.c b/tools/omfwd.c index cbfc36a4..96b365a0 100644 --- a/tools/omfwd.c +++ b/tools/omfwd.c @@ -515,7 +515,6 @@ finalize_it: BEGINparseSelectorAct uchar *q; int i; - int bErr; rsRetVal localRet; struct addrinfo; TCPFRAMINGMODE tcp_framing = TCP_FRAMING_OCTET_STUFFING; @@ -638,7 +637,6 @@ CODE_STD_STRING_REQUESTparseSelectorAct(1) } /* now skip to template */ - bErr = 0; while(*p && *p != ';' && *p != '#' && !isspace((int) *p)) ++p; /*JUST SKIP*/ diff --git a/tools/omusrmsg.c b/tools/omusrmsg.c index e61751dc..768baca7 100644 --- a/tools/omusrmsg.c +++ b/tools/omusrmsg.c @@ -249,7 +249,6 @@ static rsRetVal wallmsg(uchar* pMsg, instanceData *pData) } } close(ttyf); - ttyf = -1; } } diff --git a/tools/syslogd.c b/tools/syslogd.c index a03dcf0e..12d94e9a 100644 --- a/tools/syslogd.c +++ b/tools/syslogd.c @@ -1190,7 +1190,6 @@ int parseLegacySyslogMsg(msg_t *pMsg, int flags) { uchar *p2parse; int lenMsg; - int bTAGCharDetected; int i; /* general index for parsing */ uchar bufParseTAG[CONF_TAG_MAXSIZE]; uchar bufParseHOSTNAME[CONF_HOSTNAME_MAXSIZE]; @@ -1251,7 +1250,6 @@ int parseLegacySyslogMsg(msg_t *pMsg, int flags) * rgerhards, 2009-06-23: and I now have extended this logic to every character * that is not a valid hostname. */ - bTAGCharDetected = 0; if(lenMsg > 0 && flags & PARSE_HOSTNAME) { i = 0; while(i < lenMsg && (isalnum(p2parse[i]) || p2parse[i] == '.' || p2parse[i] == '.' -- cgit