Merge branch 'v5-stable' into v5-devel

Conflicts:
	ChangeLog
	configure.ac
	doc/manual.html

20 files changed, 141 insertions, 98 deletions

diff --git a/ChangeLog b/ChangeLog
index d0661256..275aeb22 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -48,7 +48,21 @@ Version 5.7.0  [V5-DEVEL] (rgerhards), 2010-09-16
     thanks to Lennart Poettering for this patch
   * sd-systemd API added as part of rsyslog runtime library
 ---------------------------------------------------------------------------
-Version 5.6.2  [V5-STABLE] (rgerhards), 2010-11-??
+Version 5.6.3  [V5-STABLE] (rgerhards), 2010-12-??
+- bugfix: action processor released mememory too early, resulting in
+  potential issue in retry cases (but very unlikely due to another
+  bug, which I also fixed -- only after the fix this problem here
+  became actually visible).
+- bugfix: batch processing flagged invalid message as "bad" under some
+  circumstances
+- bugfix: unitialized variable could cause issues under extreme conditions
+  plus some minor nits. This was found after a clang static code analyzer
+  analysis (great tool, and special thanks to Marcin for telling me about
+  it!)
+- bugfix: batches which had actions in error were not properly retried in
+  all cases
+---------------------------------------------------------------------------
+Version 5.6.2  [V5-STABLE] (rgerhards), 2010-11-30
 - bugfix: compile failed on systems without epoll_create1()
   Thanks to David Hill for providing a fix.
 - bugfix: atomic increment for msg object may not work correct on all
@@ -627,11 +641,23 @@ Version 4.6.5  [v4-stable] (rgerhards), 2010-??-??
   in a tight loop, effectively disabling functionality and bearing the
   risk of unresponsiveness of the whole system.
   Bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=194
+---------------------------------------------------------------------------
+Version 4.6.6  [v4-stable] (rgerhards), 2010-11-??
 - bugfix: a couple of problems that imfile had on some platforms, namely
   Ubuntu (not their fault, but occured there)
 - 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
+  in a tight loop, effectively disabling functionality and bearing the
+  risk of unresponsiveness of the whole system.
+  Bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=194
 ---------------------------------------------------------------------------
 Version 4.6.4  [v4-stable] (rgerhards), 2010-08-05
 - bugfix: zero-sized (empty) messages were processed by imtcp
@@ -1259,6 +1285,9 @@ version before switching to this one.
 - bugfix: memory leak in ompgsql
   Thanks to Ken for providing the patch
 ---------------------------------------------------------------------------
+Version 3.22.4 [v3-stable] (rgerhards), 2010-??-??
+- improved some code based on clang static analyzer results
+---------------------------------------------------------------------------
 Version 3.22.3 [v3-stable] (rgerhards), 2010-11-24
 - bugfix(important): problem in TLS handling could cause rsyslog to loop
   in a tight loop, effectively disabling functionality and bearing the
diff --git a/action.c b/action.c
index 0d298673..4bf8ba04 100644
--- a/action.c
+++ b/action.c
@@ -655,30 +655,34 @@ rsRetVal actionDbgPrint(action_t *pThis)
 /* prepare the calling parameters for doAction()
  * rgerhards, 2009-05-07
  */
-static rsRetVal prepareDoActionParams(action_t *pAction, msg_t *pMsg, uchar **ppMsgs, size_t *lenMsgs)
+static rsRetVal prepareDoActionParams(action_t *pAction, batch_obj_t *pElem)
 {
 	int i;
+	msg_t *pMsg;
 	DEFiRet;
 
 	ASSERT(pAction != NULL);
+	ASSERT(pElem != NULL);
 
+	pMsg = (msg_t*) pElem->pUsrp;
 	/* here we must loop to process all requested strings */
 	for(i = 0 ; i < pAction->iNumTpls ; ++i) {
 		switch(pAction->eParamPassing) {
 			case ACT_STRING_PASSING:
-				CHKiRet(tplToString(pAction->ppTpl[i], pMsg, &(ppMsgs[i]), &lenMsgs[i]));
+				CHKiRet(tplToString(pAction->ppTpl[i], pMsg, &(pElem->staticActStrings[i]),
+					&pElem->staticLenStrings[i]));
+				pElem->staticActParams[i] = pElem->staticActStrings[i];
 				break;
 			case ACT_ARRAY_PASSING:
-				CHKiRet(tplToArray(pAction->ppTpl[i], pMsg, (uchar***) &(ppMsgs[i])));
+				CHKiRet(tplToArray(pAction->ppTpl[i], pMsg, (uchar***) &(pElem->staticActParams[i])));
 				break;
 			case ACT_MSG_PASSING:
-				/* we abuse the uchar* ptr, it now actually is a void*, but we can not
-				 * change that other than by chaning the interface, what we don't like...
-				 */
-				ppMsgs[i] = (void*) pMsg;
-				lenMsgs[i] = 0; /* init for *next* action */
+				pElem->staticActParams[i] = (void*) pMsg;
+				break;
+			default:dbgprintf("software bug/error: unknown pAction->eParamPassing %d in prepareDoActionParams\n",
+					   (int) pAction->eParamPassing);
+				assert(0); /* software bug if this happens! */
 				break;
-			default:assert(0); /* software bug if this happens! */
 		}
 	}
 
@@ -687,26 +691,56 @@ finalize_it:
 }
 
 
-/* cleanup doAction calling parameters
- * rgerhards, 2009-05-07
+/* free a batches ressources, but not string buffers (because they will
+ * most probably be reused). String buffers are only deleted upon final
+ * destruction of the batch.
+ * This function here must be called only when the batch is actually no
+ * longer used, also not for retrying actions or such. It invalidates
+ * buffers.
+ * rgerhards, 2010-12-17
  */
-static rsRetVal cleanupDoActionParams(action_t *pAction, uchar ***ppMsgs)
+static rsRetVal releaseBatch(action_t *pAction, batch_t *pBatch)
 {
 	int iArr;
-	int i;
+	int i, j;
+	batch_obj_t *pElem;
+	uchar ***ppMsgs;
 	DEFiRet;
 
 	ASSERT(pAction != NULL);
 
-	for(i = 0 ; i < pAction->iNumTpls ; ++i) {
-		if(((uchar**)ppMsgs)[i] != NULL) {
-			iArr = 0;
-			while((((uchar***)ppMsgs)[i][iArr]) != NULL) {
-				d_free(((uchar ***)ppMsgs)[i][iArr++]);
-				((uchar ***)ppMsgs)[i][iArr++] = NULL;
+	for(i = 0 ; i < batchNumMsgs(pBatch) && !*(pBatch->pbShutdownImmediate) ; ++i) {
+		pElem = &(pBatch->pElem[i]);
+		if(pElem->bFilterOK && pElem->state != BATCH_STATE_DISC) {
+			switch(pAction->eParamPassing) {
+			case ACT_ARRAY_PASSING:
+				ppMsgs = (uchar***) pElem->staticActParams;
+				for(i = 0 ; i < pAction->iNumTpls ; ++i) {
+					if(((uchar**)ppMsgs)[i] != NULL) {
+						iArr = 0;
+						while(ppMsgs[i][iArr] != NULL) {
+							d_free(ppMsgs[i][iArr++]);
+							ppMsgs[i][iArr++] = NULL;
+						}
+						d_free(((uchar**)ppMsgs)[i]);
+						((uchar**)ppMsgs)[i] = NULL;
+					}
+				}
+				break;
+			case ACT_STRING_PASSING:
+			case ACT_MSG_PASSING:
+				/* nothing to do in that case */
+				/* TODO ... and yet we do something ;) This is considered not
+				 * really needed, but I was not bold enough to remove that while
+				 * fixing the stable. It should be removed in a devel version
+				 * soon (I really don't see a reason why we would need it).
+				 * rgerhards, 2010-12-16
+				 */
+				for(j = 0 ; j < pAction->iNumTpls ; ++j) {
+					((uchar**)pElem->staticActParams)[j] = NULL;
+				}
+				break;
 			}
-			d_free(((uchar**)ppMsgs)[i]);
-			((uchar**)ppMsgs)[i] = NULL;
 		}
 	}
 
@@ -720,7 +754,6 @@ static rsRetVal cleanupDoActionParams(action_t *pAction, uchar ***ppMsgs)
 rsRetVal
 actionCallDoAction(action_t *pThis, msg_t *pMsg, void *actParams)
 {
-	int i;
 	DEFiRet;
 
 	ASSERT(pThis != NULL);
@@ -729,10 +762,7 @@ actionCallDoAction(action_t *pThis, msg_t *pMsg, void *actParams)
 	DBGPRINTF("entering actionCalldoAction(), state: %s\n", getActStateName(pThis));
 
 	pThis->bHadAutoCommit = 0;
-//d_pthread_mutex_lock(&pThis->mutActExec);
-//pthread_cleanup_push(mutexCancelCleanup, &pThis->mutActExec);
 	iRet = pThis->pMod->mod.om.doAction(actParams, pMsg->msgFlags, pThis->pModData);
-//pthread_cleanup_pop(1); /* unlock mutex */
 	switch(iRet) {
 		case RS_RET_OK:
 			actionCommitted(pThis);
@@ -761,27 +791,6 @@ actionCallDoAction(action_t *pThis, msg_t *pMsg, void *actParams)
 	iRet = getReturnCode(pThis);
 
 finalize_it:
-
-	/* we need to cleanup the batches string buffers if they have been used
-	 * in a non-standard way. -- rgerhards, 2010-06-15
-	 * Note that we may do this at the batch level, this would provide a bit
-	 * more concurrency (TODO).
-	 */
-	switch(pThis->eParamPassing) {
-	case ACT_STRING_PASSING:
-		/* nothing to do in that case */
-		break;
-	case ACT_ARRAY_PASSING:
-		cleanupDoActionParams(pThis, actParams); /* iRet ignored! */
-		break;
-	case ACT_MSG_PASSING:
-		/* nothing to do in that case */
-		for(i = 0 ; i < pThis->iNumTpls ; ++i) {
-			((uchar**)actParams)[i] = NULL;
-		}
-		break;
-	}
-
 	RETiRet;
 }
 
@@ -944,10 +953,12 @@ submitBatch(action_t *pAction, batch_t *pBatch, int nElem)
 	int i;
 	int bDone;
 	rsRetVal localRet;
+	int wasDoneTo;
 	DEFiRet;
 
 	assert(pBatch != NULL);
 
+	wasDoneTo = pBatch->iDoneUpTo;
 	bDone = 0;
 	do {
 		localRet = tryDoAction(pAction, pBatch, &nElem);
@@ -971,7 +982,7 @@ submitBatch(action_t *pAction, batch_t *pBatch, int nElem)
 			; /* do nothing, this will retry the full batch */
 		} else if(localRet == RS_RET_ACTION_FAILED) {
 			/* in this case, everything not yet committed is BAD */
-			for(i = pBatch->iDoneUpTo ; i < nElem ; ++i) {
+			for(i = pBatch->iDoneUpTo ; i < wasDoneTo + nElem ; ++i) {
 				if(   pBatch->pElem[i].state != BATCH_STATE_DISC
 				   && pBatch->pElem[i].state != BATCH_STATE_COMM ) {
 					pBatch->pElem[i].state = BATCH_STATE_BAD;
@@ -981,7 +992,7 @@ submitBatch(action_t *pAction, batch_t *pBatch, int nElem)
 			bDone = 1;
 		} else {
 			if(nElem == 1) {
-				batchSetElemState(pBatch, i, BATCH_STATE_BAD);
+				batchSetElemState(pBatch, pBatch->iDoneUpTo, BATCH_STATE_BAD);
 				bDone = 1;
 			} else {
 				/* retry with half as much. Depth is log_2 batchsize, so recursion is not too deep */
@@ -1021,8 +1032,7 @@ prepareBatch(action_t *pAction, batch_t *pBatch)
 		pElem = &(pBatch->pElem[i]);
 		if(pElem->bFilterOK && pElem->state != BATCH_STATE_DISC) {
 			pElem->state = BATCH_STATE_RDY;
-			prepareDoActionParams(pAction, (msg_t*) pElem->pUsrp,
-				 	      (uchar**) &(pElem->staticActParams), pElem->staticLenParams);
+			prepareDoActionParams(pAction, pElem);
 		}
 	}
 	RETiRet;
@@ -1055,6 +1065,7 @@ static rsRetVal
 processBatchMain(action_t *pAction, batch_t *pBatch, int *pbShutdownImmediate)
 {
 	int *pbShutdownImmdtSave;
+	rsRetVal localRet;
 	DEFiRet;
 
 	assert(pBatch != NULL);
@@ -1076,6 +1087,16 @@ processBatchMain(action_t *pAction, batch_t *pBatch, int *pbShutdownImmediate)
 
 	pthread_cleanup_pop(1); /* unlock mutex */
 
+	/* even if processAction failed, we need to release the batch (else we
+	 * have a memory leak). So we do this first, and then check if we need to
+	 * return an error code. If so, the code from processAction has priority.
+	 * rgerhards, 2010-12-17
+	 */
+	localRet = releaseBatch(pAction, pBatch);
+
+	if(iRet == RS_RET_OK)
+		iRet = localRet;
+
 finalize_it:
 	pBatch->pbShutdownImmediate = pbShutdownImmdtSave;
 	RETiRet;
@@ -1360,7 +1381,7 @@ doSubmitToActionQNotAllMarkBatch(action_t *pAction, batch_t *pBatch)
 	int i;
 	int bProcessMarkMsgs = 0;
 	int bModifiedFilter;
-	sbool FilterSave[128];
+	sbool FilterSave[1024];
 	sbool *pFilterSave;
 	DEFiRet;
 
@@ -1405,6 +1426,7 @@ doSubmitToActionQNotAllMarkBatch(action_t *pAction, batch_t *pBatch)
 	if(bModifiedFilter) {
 		/* in this case, we need to restore previous state */
 		for(i = 0 ; i < batchNumMsgs(pBatch) ; ++i) {
+			/* note: clang static code analyzer reports a false positive below */
 			pBatch->pElem[i].bFilterOK = pFilterSave[i];
 		}
 	}
diff --git a/doc/rsyslog_conf_filter.html b/doc/rsyslog_conf_filter.html
index 63c29817..34839616 100644
--- a/doc/rsyslog_conf_filter.html
+++ b/doc/rsyslog_conf_filter.html
@@ -85,12 +85,12 @@ selector field is capable to overwrite the preceding ones. Using this
 behavior you can exclude some priorities from the pattern.</p>
 <p>Rsyslogd has a syntax extension to the original BSD source,
 that makes its use more intuitively. You may precede every priority
-with an equation sign ("='') to specify only this single priority and
+with an equals sign ("='') to specify only this single priority and
 not any of the above. You may also (both is valid, too) precede the
 priority with an exclamation mark ("!'') to ignore all that
 priorities, either exact this one or this and any higher priority. If
 you use both extensions than the exclamation mark must occur before the
-equation sign, just use it intuitively.</p>
+equals sign, just use it intuitively.</p>
 <h3>Property-Based Filters</h3>
 <p>Property-based filters are unique to rsyslogd. They allow to
 filter on any property, like HOSTNAME, syslogtag and msg. A list of all
diff --git a/plugins/imklog/ksym.c b/plugins/imklog/ksym.c
index 058b2cfa..ebaec011 100644
--- a/plugins/imklog/ksym.c
+++ b/plugins/imklog/ksym.c
@@ -651,8 +651,7 @@ static void FreeSymbols(void)
  **************************************************************************/
 extern char *ExpandKadds(char *line, char *el)
 {
-	auto char	dlm,
-			*kp,
+	auto char	*kp,
 			*sl = line,
 			*elp = el,
 			*symbol;
@@ -782,7 +781,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/plugins/omtesting/omtesting.c b/plugins/omtesting/omtesting.c
index 9442f691..c474bb41 100644
--- a/plugins/omtesting/omtesting.c
+++ b/plugins/omtesting/omtesting.c
@@ -188,8 +188,10 @@ CODESTARTdoAction
 			break;
 		case MD_RANDFAIL:
 			iRet = doRandFail();
+			break;
 		case MD_ALWAYS_SUSPEND:
 			iRet = RS_RET_SUSPENDED;
+			break;
 	}
 
 	if(iRet == RS_RET_OK && pData->bEchoStdout) {
diff --git a/runtime/batch.h b/runtime/batch.h
index 68f48d8b..d0504f2b 100644
--- a/runtime/batch.h
+++ b/runtime/batch.h
@@ -53,9 +53,11 @@ struct batch_obj_s {
 	 */
 	sbool bFilterOK;	/* work area for filter processing (per action, reused!) */
 	sbool bPrevWasSuspended;
-	void *staticActParams[CONF_OMOD_NUMSTRINGS_MAXSIZE];
+	/* following are caches to save allocs if not absolutely necessary */
+	uchar *staticActStrings[CONF_OMOD_NUMSTRINGS_MAXSIZE]; /**< for strings */
 				/* a cache to save malloc(), if not absolutely necessary */
-	size_t staticLenParams[CONF_OMOD_NUMSTRINGS_MAXSIZE];
+	void *staticActParams[CONF_OMOD_NUMSTRINGS_MAXSIZE]; /**< for anything else */
+	size_t staticLenStrings[CONF_OMOD_NUMSTRINGS_MAXSIZE];
 				/* and the same for the message length (if used) */
 	/* end action work variables */
 };
@@ -152,7 +154,10 @@ batchFree(batch_t *pBatch) {
 	int j;
 	for(i = 0 ; i < pBatch->maxElem ; ++i) {
 		for(j = 0 ; j < CONF_OMOD_NUMSTRINGS_MAXSIZE ; ++j) {
-			free(pBatch->pElem[i].staticActParams[j]);
+			/* staticActParams MUST be freed immediately (if required),
+			 * so we do not need to do that!
+			 */
+			free(pBatch->pElem[i].staticActStrings[j]);
 		}
 	}
 	free(pBatch->pElem);
diff --git a/runtime/cfsysline.c b/runtime/cfsysline.c
index 5df8e64c..037e9f84 100644
--- a/runtime/cfsysline.c
+++ b/runtime/cfsysline.c
@@ -953,8 +953,6 @@ finalize_it:
  */
 void dbgPrintCfSysLineHandlers(void)
 {
-	DEFiRet;
-
 	cslCmd_t *pCmd;
 	cslCmdHdlr_t *pCmdHdlr;
 	linkedListCookie_t llCookieCmd;
@@ -963,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);
@@ -976,7 +974,6 @@ void dbgPrintCfSysLineHandlers(void)
 		}
 	}
 	dbgprintf("\n");
-	ENDfunc
 }
 
 
diff --git a/runtime/conf.c b/runtime/conf.c
index d41f2950..529142ed 100644
--- a/runtime/conf.c
+++ b/runtime/conf.c
@@ -1084,7 +1084,7 @@ static rsRetVal cflineDoAction(uchar **p, action_t **ppAction)
 	DEFiRet;
 	modInfo_t *pMod;
 	omodStringRequest_t *pOMSR;
-	action_t *pAction;
+	action_t *pAction = NULL;
 	void *pModData;
 
 	ASSERT(p != NULL);
@@ -1092,6 +1092,11 @@ static rsRetVal cflineDoAction(uchar **p, action_t **ppAction)
 
 	/* loop through all modules and see if one picks up the line */
 	pMod = module.GetNxtType(NULL, eMOD_OUT);
+	/* Note: clang static analyzer reports that pMod mybe == NULL. However, this is
+	 * not possible, because we have the built-in output modules which are always
+	 * present. Anyhow, we guard this by an assert. -- rgerhards, 2010-12-16
+	 */
+	assert(pMod != NULL);
 	while(pMod != NULL) {
 		pOMSR = NULL;
 		iRet = pMod->mod.om.parseSelectorAct(p, &pModData, &pOMSR);
diff --git a/runtime/ctok.c b/runtime/ctok.c
index 18ddaed2..99b0e095 100644
--- a/runtime/ctok.c
+++ b/runtime/ctok.c
@@ -1,4 +1,4 @@
-/* cfgtok.c - helper class to tokenize an input stream - which surprisingly
+/* ctok.c - helper class to tokenize an input stream - which surprisingly
  * currently does not work with streams but with string. But that will
  * probably change over time ;) This class was originally written to support
  * the expression module but may evolve when (if) the expression module is
@@ -267,7 +267,7 @@ ctokGetVar(ctok_t *pThis, ctok_token_t *pToken)
 {
 	DEFiRet;
 	uchar c;
-	cstr_t *pstrVal;
+	cstr_t *pstrVal = NULL;
 
 	ISOBJ_TYPE_assert(pThis, ctok);
 	ASSERT(pToken != NULL);
diff --git a/runtime/debug.c b/runtime/debug.c
index 5818dda9..3f1c23bd 100644
--- a/runtime/debug.c
+++ b/runtime/debug.c
@@ -157,9 +157,7 @@ static pthread_key_t keyCallStack;
  */
 static void dbgMutexCancelCleanupHdlr(void *pmut)
 {
-	int ret;
-	ret = pthread_mutex_unlock((pthread_mutex_t*) pmut);
-	assert(ret == 0);
+	pthread_mutex_unlock((pthread_mutex_t*) pmut);
 }
 
 
@@ -438,14 +436,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]";
@@ -474,7 +471,7 @@ static inline void dbgMutexLockLog(pthread_mutex_t *pmut, dbgFuncDB_t *pFuncDB,
 	dbgMutLogDelEntry(pLog);
 
 	/* add "lock" entry */
-	pLog = dbgMutLogAddEntry(pmut, MUTOP_LOCK, pFuncDB, lockLn);
+	dbgMutLogAddEntry(pmut, MUTOP_LOCK, pFuncDB, lockLn);
 	dbgFuncDBAddMutexLock(pFuncDB, pmut, lockLn);
 	pthread_mutex_unlock(&mutMutLog);
 	if(bPrintMutexAction)
@@ -486,14 +483,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]";
@@ -522,7 +518,7 @@ static inline void dbgMutexTryLockLog(pthread_mutex_t *pmut, dbgFuncDB_t *pFuncD
    dbgMutLogDelEntry(pLog);
 
    /* add "lock" entry */
-   pLog = dbgMutLogAddEntry(pmut, MUTOP_LOCK, pFuncDB, lockLn);
+   dbgMutLogAddEntry(pmut, MUTOP_LOCK, pFuncDB, lockLn);
    dbgFuncDBAddMutexLock(pFuncDB, pmut, lockLn);
    pthread_mutex_unlock(&mutMutLog);
    if(bPrintMutexAction)
diff --git a/runtime/msg.c b/runtime/msg.c
index 82565f18..e8be79db 100644
--- a/runtime/msg.c
+++ b/runtime/msg.c
@@ -1546,7 +1546,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:
@@ -2976,7 +2976,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;
@@ -2991,7 +2990,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 40374ae1..d3644976 100644
--- a/runtime/parser.c
+++ b/runtime/parser.c
@@ -429,14 +429,12 @@ ParsePRI(msg_t *pMsg)
 	int pri;
 	uchar *msg;
 	int lenMsg;
-	int iPriText;
 	DEFiRet;
 
 	/* pull PRI */
 	lenMsg = pMsg->iLenRawMsg;
 	msg = pMsg->pszRawMsg;
 	pri = DEFUPRI;
-	iPriText = 0;
 	if(pMsg->msgFlags & NO_PRI_IN_RAW) {
 		/* In this case, simply do so as if the pri would be right at top */
 		MsgSetAfterPRIOffs(pMsg, 0);
diff --git a/runtime/queue.c b/runtime/queue.c
index 387c15c2..e4922f37 100644
--- a/runtime/queue.c
+++ b/runtime/queue.c
@@ -1131,7 +1131,7 @@ cancelWorkers(qqueue_t *pThis)
 		 * done when *no* worker is running. So time for a shutdown... -- rgerhards, 2009-05-28
 		 */
 		DBGOPRINT((obj_t*) pThis, "checking to see if main queue DA worker pool needs to be cancelled\n");
-		iRetLocal = wtpCancelAll(pThis->pWtpDA); /* returns immediately if all threads already have terminated */
+		wtpCancelAll(pThis->pWtpDA); /* returns immediately if all threads already have terminated */
 	}
 
 	RETiRet;
diff --git a/template.c b/template.c
index 06949e45..5aaf1bcb 100644
--- a/template.c
+++ b/template.c
@@ -86,9 +86,9 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t *
 	DEFiRet;
 	struct templateEntry *pTpe;
 	size_t iBuf;
-	unsigned short bMustBeFreed;
+	unsigned short bMustBeFreed = 0;
 	uchar *pVal;
-	size_t iLenVal;
+	size_t iLenVal = 0;
 
 	assert(pTpl != NULL);
 	assert(pMsg != NULL);
@@ -1046,7 +1046,6 @@ void tplDeleteAll(void)
 {
 	struct template *pTpl, *pTplDel;
 	struct templateEntry *pTpe, *pTpeDel;
-	rsRetVal iRetLocal;
 	BEGINfunc
 
 	pTpl = tplRoot;
@@ -1069,7 +1068,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));
 					}
 				}
@@ -1095,7 +1094,6 @@ void tplDeleteNew(void)
 {
 	struct template *pTpl, *pTplDel;
 	struct templateEntry *pTpe, *pTpeDel;
-	rsRetVal iRetLocal;
 
 	BEGINfunc
 
@@ -1124,7 +1122,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 4064fc7a..04ccb8d3 100644
--- a/threads.c
+++ b/threads.c
@@ -211,7 +211,6 @@ rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), rsRetVal(*afterRun)(thrdI
 {
 	DEFiRet;
 	thrdInfo_t *pThis;
-	int i;
 
 	assert(thrdMain != NULL);
 
@@ -220,7 +219,7 @@ rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), rsRetVal(*afterRun)(thrdI
 	pThis->pUsrThrdMain = thrdMain;
 	pThis->pAfterRun = afterRun;
 	pThis->bNeedsCancel = bNeedsCancel;
-	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 78f2bf8e..8c507a56 100644
--- a/tools/omfile.c
+++ b/tools/omfile.c
@@ -404,7 +404,6 @@ prepareFile(instanceData *pData, uchar *newFileName)
 		}
 	} else {
 		/* 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 481fabab..487bb35a 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/rsyslog.conf.5 b/tools/rsyslog.conf.5
index e8a4ab92..e17da974 100644
--- a/tools/rsyslog.conf.5
+++ b/tools/rsyslog.conf.5
@@ -200,11 +200,11 @@ to overwrite the preceding ones. Using this behavior you can exclude some
 priorities from the pattern.
 
 Rsyslogd has a syntax extension to the original BSD source, that makes its use
-more intuitively. You may precede every priority with an equation sign ('=') to
+more intuitively. You may precede every priority with an equals sign ('=') to
 specify only this single priority and not any of the above. You may also (both
 is valid, too) precede the priority with an exclamation mark ('!') to ignore
 all that priorities, either exact this one or this and any higher priority. If
-you use both extensions than the exclamation mark must occur before the equation
+you use both extensions than the exclamation mark must occur before the equals
 sign, just use it intuitively.
 
 .SH ACTIONS
diff --git a/tools/syslogd.c b/tools/syslogd.c
index 70d4cf2e..3c3c2f79 100644
--- a/tools/syslogd.c
+++ b/tools/syslogd.c
@@ -2592,7 +2592,7 @@ int realMain(int argc, char **argv)
 		}
 	}
 
-	if ((argc -= optind))
+	if(argc - optind)
 		usage();
 
 	DBGPRINTF("rsyslogd %s startup, compatibility mode %d, module path '%s', cwd:%s\n",