optimized template string generation

7 files changed, 125 insertions, 91 deletions

diff --git a/action.c b/action.c
index 22755e86..d214e808 100644
--- a/action.c
+++ b/action.c
@@ -178,6 +178,7 @@ actionResetQueueParams(void)
  */
 rsRetVal actionDestruct(action_t *pThis)
 {
+	int i;
 	DEFiRet;
 	ASSERT(pThis != NULL);
 
@@ -195,7 +196,33 @@ rsRetVal actionDestruct(action_t *pThis)
 	pthread_mutex_destroy(&pThis->mutActExec);
 	d_free(pThis->pszName);
 	d_free(pThis->ppTpl);
+
+	/* message ptr cleanup */
+	for(i = 0 ; i < pThis->iNumTpls ; ++i) {
+		if(pThis->ppMsgs[i] != NULL) {
+			switch(pThis->eParamPassing) {
+			case ACT_ARRAY_PASSING:
+#if 0 /* later! */
+				iArr = 0;
+				while(((char **)pThis->ppMsgs[i])[iArr] != NULL) {
+					d_free(((char **)pThis->ppMsgs[i])[iArr++]);
+					((char **)pThis->ppMsgs[i])[iArr++] = NULL;
+				}
+				d_free(pThis->ppMsgs[i]);
+				pThis->ppMsgs[i] = NULL;
+#endif
+				break;
+			case ACT_STRING_PASSING:
+				d_free(pThis->ppMsgs[i]);
+				break;
+			default:
+				assert(0);
+			}
+		}
+	}
 	d_free(pThis->ppMsgs);
+	d_free(pThis->lenMsgs);
+
 	d_free(pThis);
 	
 	RETiRet;
@@ -440,7 +467,7 @@ actionCallDoAction(action_t *pAction, msg_t *pMsg)
 	for(i = 0 ; i < pAction->iNumTpls ; ++i) {
 		switch(pAction->eParamPassing) {
 			case ACT_STRING_PASSING:
-				CHKiRet(tplToString(pAction->ppTpl[i], pMsg, &(pAction->ppMsgs[i])));
+				CHKiRet(tplToString(pAction->ppTpl[i], pMsg, &(pAction->ppMsgs[i]), &(pAction->lenMsgs[i])));
 				break;
 			case ACT_ARRAY_PASSING:
 				CHKiRet(tplToArray(pAction->ppTpl[i], pMsg, (uchar***) &(pAction->ppMsgs[i])));
@@ -513,7 +540,6 @@ finalize_it:
 				pAction->ppMsgs[i] = NULL;
 				break;
 			case ACT_STRING_PASSING:
-				d_free(pAction->ppMsgs[i]);
 				break;
 			default:
 				assert(0);
@@ -727,32 +753,13 @@ finalize_it:
 }
 
 
-/* call the configured action. Does all necessary housekeeping.
- * rgerhards, 2007-08-01
- * FYI: currently, this function is only called from the queue
- * consumer. So we (conceptually) run detached from the input
- * threads (which also means we may run much later than when the
- * message was generated).
+/* helper to actonCallAction, mostly needed because of this damn
+ * pthread_cleanup_push() POSIX macro...
  */
-#pragma GCC diagnostic ignored "-Wempty-body"
-rsRetVal
-actionCallAction(action_t *pAction, msg_t *pMsg)
+static rsRetVal
+doActionCallAction(action_t *pAction, msg_t *pMsg)
 {
 	DEFiRet;
-	int iCancelStateSave;
-
-	ISOBJ_TYPE_assert(pMsg, msg);
-	ASSERT(pAction != NULL);
-
-	/* Make sure nodbody else modifies/uses this action object. Right now, this
-         * is important because of "message repeated n times" processing and potentially
-	 * multiple worker threads. -- rgerhards, 2007-12-11
- 	 */
-	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave);
-	LockObj(pAction);
-	pthread_cleanup_push(mutexCancelCleanup, pAction->Sync_mut);
-	pthread_setcancelstate(iCancelStateSave, NULL);
-
 	/* first, we need to check if this is a disabled
 	 * entry. If so, we must not further process it.
 	 * rgerhards 2005-09-26
@@ -813,10 +820,43 @@ actionCallAction(action_t *pAction, msg_t *pMsg)
 	}
 
 finalize_it:
-	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave);
-	UnlockObj(pAction);
-	pthread_cleanup_pop(0); /* remove mutex cleanup handler */
-	pthread_setcancelstate(iCancelStateSave, NULL);
+	RETiRet;
+}
+
+/* call the configured action. Does all necessary housekeeping.
+ * rgerhards, 2007-08-01
+ * FYI: currently, this function is only called from the queue
+ * consumer. So we (conceptually) run detached from the input
+ * threads (which also means we may run much later than when the
+ * message was generated).
+ */
+#pragma GCC diagnostic ignored "-Wempty-body"
+rsRetVal
+actionCallAction(action_t *pAction, msg_t *pMsg)
+{
+	DEFiRet;
+	int iCancelStateSave;
+
+	ISOBJ_TYPE_assert(pMsg, msg);
+	ASSERT(pAction != NULL);
+
+	/* We need to lock the mutex only for repeated line processing. 
+	 * rgerhards, 2009-06-19
+	 */
+	if(pAction->f_ReduceRepeated == 1) {
+		pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave);
+		LockObj(pAction);
+		pthread_cleanup_push(mutexCancelCleanup, pAction->Sync_mut);
+		pthread_setcancelstate(iCancelStateSave, NULL);
+		iRet = doActionCallAction(pAction, pMsg);
+		pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave);
+		UnlockObj(pAction);
+		pthread_cleanup_pop(0); /* remove mutex cleanup handler */
+		pthread_setcancelstate(iCancelStateSave, NULL);
+	} else {
+		iRet = doActionCallAction(pAction, pMsg);
+	}
+
 	RETiRet;
 }
 #pragma GCC diagnostic warning "-Wempty-body"
@@ -903,8 +943,9 @@ addAction(action_t **ppAction, modInfo_t *pMod, void *pModData, omodStringReques
 	 */
 	if(pAction->iNumTpls > 0) {
 		/* we first need to create the template pointer array */
-		CHKmalloc(pAction->ppTpl = (struct template *)calloc(pAction->iNumTpls, sizeof(struct template *)));
-		CHKmalloc(pAction->ppMsgs = (uchar**) malloc(pAction->iNumTpls * sizeof(uchar *)));
+		CHKmalloc(pAction->ppTpl = (struct template **)calloc(pAction->iNumTpls, sizeof(struct template *)));
+		CHKmalloc(pAction->ppMsgs = (uchar**) calloc(pAction->iNumTpls, sizeof(uchar *)));
+		CHKmalloc(pAction->lenMsgs = (size_t*) calloc(pAction->iNumTpls, sizeof(size_t)));
 	}
 	
 	for(i = 0 ; i < pAction->iNumTpls ; ++i) {
diff --git a/action.h b/action.h
index a6c02a8b..579a1215 100644
--- a/action.h
+++ b/action.h
@@ -75,6 +75,7 @@ struct action_s {
 	pthread_mutex_t mutActExec; /* mutex to guard actual execution of doAction for single-threaded modules */
 	uchar *pszName;		/* action name (for documentation) */
 	uchar **ppMsgs;		/* pointer to action-calling parameters (kept in structure to save alloc() time!) */
+	size_t *lenMsgs;	/* length of message in ppMsgs */
 };
 typedef struct action_s action_t;
 
diff --git a/runtime/datetime.c b/runtime/datetime.c
index b5514e7c..8f4bfa10 100644
--- a/runtime/datetime.c
+++ b/runtime/datetime.c
@@ -556,7 +556,7 @@ int formatTimestampToMySQL(struct syslogTime *ts, char* pBuf, size_t iLenDst)
 	 */
 	assert(ts != NULL);
 	assert(pBuf != NULL);
-	assert(iLenDst < 15);
+	assert(iLenDst >= 15);
 
 	pBuf[0] = (ts->year / 1000) % 10 + '0';
 	pBuf[1] = (ts->year / 100) % 10 + '0';
@@ -582,7 +582,7 @@ int formatTimestampToPgSQL(struct syslogTime *ts, char *pBuf, size_t iLenDst)
 	/* see note in formatTimestampToMySQL, applies here as well */
 	assert(ts != NULL);
 	assert(pBuf != NULL);
-	assert(iLenDst < 20);
+	assert(iLenDst >= 20);
 
 	pBuf[0] = (ts->year / 1000) % 10 + '0';
 	pBuf[1] = (ts->year / 100) % 10 + '0';
@@ -666,7 +666,7 @@ int formatTimestamp3339(struct syslogTime *ts, char* pBuf, size_t iLenBuf)
 	BEGINfunc
 	assert(ts != NULL);
 	assert(pBuf != NULL);
-	assert(iLenBuf < 33);
+	assert(iLenBuf >= 33);
 
 	/* start with fixed parts */
 	/* year yyyy */
@@ -741,7 +741,7 @@ int formatTimestamp3164(struct syslogTime *ts, char* pBuf, size_t iLenBuf)
 	assert(ts != NULL);
 	assert(pBuf != NULL);
 	
-	assert(iLenBuf < 16);
+	assert(iLenBuf >= 16);
 
 	pBuf[0] = monthNames[(ts->month - 1)% 12][0];
 	pBuf[1] = monthNames[(ts->month - 1) % 12][1];
diff --git a/runtime/stringbuf.c b/runtime/stringbuf.c
index e7fa8c41..d3ddf33a 100644
--- a/runtime/stringbuf.c
+++ b/runtime/stringbuf.c
@@ -461,14 +461,11 @@ cstrFinalize(cstr_t *pThis)
 	DEFiRet;
 	rsCHECKVALIDOBJECT(pThis, OIDrsCStr);
 	
-	assert(pThis->bIsFinalized == 0);
-
 	if(pThis->iStrLen > 0) {
 		/* terminate string only if one exists */
 		CHKiRet(cstrAppendChar(pThis, '\0'));
 		--pThis->iStrLen;	/* do NOT count the \0 byte */
 	}
-	pThis->bIsFinalized = 1;
 
 finalize_it:
 	RETiRet;
diff --git a/runtime/stringbuf.h b/runtime/stringbuf.h
index b4fc3f3c..9d2e7865 100644
--- a/runtime/stringbuf.h
+++ b/runtime/stringbuf.h
@@ -48,7 +48,7 @@ typedef struct cstr_s
 	size_t iBufSize;	/**< current maximum size of the string buffer */
 	size_t iStrLen;		/**< length of the string in characters. */
 	unsigned short iAllocIncrement;	/**< the amount of bytes the string should be expanded if it needs to */
-	bool bIsFinalized;	/**< is this object finished and ready for use? (a debug aid, may be removed later TODO 2009-06-16) */
+	bool bIsForeignBuf;	/**< is pBuf a buffer provided by someone else? */
 } cstr_t;
 
 
diff --git a/template.c b/template.c
index 93d0539c..43a54d98 100644
--- a/template.c
+++ b/template.c
@@ -49,59 +49,61 @@ static struct template *tplLastStatic = NULL; /* last static element of the temp
 
 
 
-/* This functions converts a template into a string. It should
- * actually be in template.c, but this requires larger re-structuring
- * of the code (because all the property-access functions are static
- * to this module). I have placed it next to the iov*() functions, as
- * it is somewhat similiar in what it does.
- *
- * The function takes a pointer to a template and a pointer to a msg object.
- * It the creates a string based on the template definition. A pointer
- * to that string is returned to the caller. The caller MUST FREE that
- * pointer when it is no longer needed. If the function fails, NULL
- * is returned.
- * If memory allocation fails in this function, we silently return
- * NULL. The reason is that we can not do anything against it. And
- * if we raise an alert, the memory situation might become even
- * worse. So we prefer to let the caller deal with it.
- * rgerhards, 2007-07-03
+/* helper to tplToString, extends buffer */
+#define ALLOC_INC 128
+static inline rsRetVal ExtendBuf(uchar **pBuf, size_t *pLenBuf, size_t iMinSize)
+{
+	uchar *pNewBuf;
+	size_t iNewSize;
+	DEFiRet;
+
+	iNewSize = (iMinSize / ALLOC_INC + 1) * ALLOC_INC;
+	CHKmalloc(pNewBuf = (uchar*) realloc(*pBuf, iNewSize));
+	*pBuf = pNewBuf;
+	*pLenBuf = iNewSize;
+dbgprintf("extend buf to at least %ld, done %ld\n", iMinSize, iNewSize);
+
+finalize_it:
+	RETiRet;
+}
+
+
+/* This functions converts a template into a string.
  *
- * rgerhards, 2007-09-05: I changed the interface to use the standard iRet
- * "calling sequence". This greatly eases complexity when it comes to handling
- * errors in called modules (plus, it is much nicer).
+ * The function takes a pointer to a template and a pointer to a msg object
+ * as well as a pointer to an output buffer and its size. Note that the output
+ * buffer pointer may be NULL, size 0, in which case a new one is allocated.
+ * The outpub buffer is grown as required. It is the caller's duty to free the
+ * buffer when it is done. Note that it is advisable to reuse memory, as this
+ * offers big performance improvements.
+ * rewritten 2009-06-19 rgerhards
  */
-rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar** ppSz)
+rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t *pLenBuf)
 {
 	DEFiRet;
 	struct templateEntry *pTpe;
-	cstr_t *pCStr;
+	int iBuf;
 	unsigned short bMustBeFreed;
 	uchar *pVal;
 	size_t iLenVal;
 
 	assert(pTpl != NULL);
 	assert(pMsg != NULL);
-	assert(ppSz != NULL);
+	assert(ppBuf != NULL);
+	assert(pLenBuf != NULL);
 
 	/* loop through the template. We obtain one value
 	 * and copy it over to our dynamic string buffer. Then, we
 	 * free the obtained value (if requested). We continue this
 	 * loop until we got hold of all values.
 	 */
-	CHKiRet(cstrConstruct(&pCStr));
-
 	pTpe = pTpl->pEntryRoot;
+	iBuf = 0;
 	while(pTpe != NULL) {
 		if(pTpe->eEntryType == CONSTANT) {
-			CHKiRet_Hdlr(rsCStrAppendStrWithLen(pCStr, 
-							  (uchar *) pTpe->data.constant.pConstant,
-							  pTpe->data.constant.iLenConstant)
-							 ) {
-				dbgprintf("error %d during tplToString()\n", iRet);
-				/* it does not make sense to continue now */
-				cstrDestruct(&pCStr);
-				FINALIZE;
-			}
+			pVal = (uchar*) pTpe->data.constant.pConstant;
+			iLenVal = pTpe->data.constant.iLenConstant;
+			bMustBeFreed = 0;
 		} else 	if(pTpe->eEntryType == FIELD) {
 			pVal = (uchar*) MsgGetProp(pMsg, pTpe, NULL, &iLenVal, &bMustBeFreed);
 			/* we now need to check if we should use SQL option. In this case,
@@ -114,30 +116,23 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar** ppSz)
 				doSQLEscape(&pVal, &iLenVal, &bMustBeFreed, 1);
 			else if(pTpl->optFormatForSQL == 2)
 				doSQLEscape(&pVal, &iLenVal, &bMustBeFreed, 0);
-			/* value extracted, so lets copy */
-			CHKiRet_Hdlr(rsCStrAppendStrWithLen(pCStr, (uchar*) pVal, iLenVal)) {
-				dbgprintf("error %d during tplToString()\n", iRet);
-				/* it does not make sense to continue now */
-				cstrDestruct(&pCStr);
-				if(bMustBeFreed)
-					free(pVal);
-				FINALIZE;
-			}
-			if(bMustBeFreed)
-				free(pVal);
 		}
+		/* got source, now copy over */
+		if(iBuf + iLenVal + 1 >= *pLenBuf) /* we reserve one char for the final \0! */
+			CHKiRet(ExtendBuf(ppBuf, pLenBuf, iBuf + iLenVal + 1));
+
+		memcpy(*ppBuf + iBuf, pVal, iLenVal);
+		iBuf += iLenVal;
+
+		if(bMustBeFreed)
+			free(pVal);
+
 		pTpe = pTpe->pNext;
 	}
 
-	/* we are done with the template, now let's convert the result into a
-	 * "real" (usable) string and discard the helper structures.
-	 */
-	CHKiRet(cstrFinalize(pCStr));
-	CHKiRet(cstrConvSzStrAndDestruct(pCStr, &pVal, 0));
+	(*ppBuf)[iBuf] = '\0'; /* space was reserved above (see copy) */
 	
 finalize_it:
-	*ppSz = (iRet == RS_RET_OK) ? pVal : NULL;
-
 	RETiRet;
 }
 
diff --git a/template.h b/template.h
index 9d794f66..73ed0f84 100644
--- a/template.h
+++ b/template.h
@@ -127,7 +127,7 @@ void tplLastStaticInit(struct template *tpl);
  * rgerhards, 2007-08-06
  */
 rsRetVal tplToArray(struct template *pTpl, msg_t *pMsg, uchar*** ppArr);
-rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar** ppSz);
+rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar** ppSz, size_t *);
 rsRetVal doSQLEscape(uchar **pp, size_t *pLen, unsigned short *pbMustBeFreed, int escapeMode);
 
 rsRetVal templateInit();