cleaned up/optimized raw message handling in msg object

12 files changed, 77 insertions, 84 deletions

diff --git a/action.c b/action.c
index 8bdb6dec..08cdd6fd 100644
--- a/action.c
+++ b/action.c
@@ -675,7 +675,7 @@ actionWriteToAction(action_t *pAction)
 		datetime.getCurrTime(&(pMsg->tRcvdAt), &(pMsg->ttGenTime));
 		memcpy(&pMsg->tTIMESTAMP, &pMsg->tRcvdAt, sizeof(struct syslogTime));
 		MsgSetMSG(pMsg, (char*)szRepMsg);
-		MsgSetRawMsg(pMsg, (char*)szRepMsg);
+		MsgSetRawMsgWOSize(pMsg, (char*)szRepMsg);
 
 		pMsgSave = pAction->f_pMsg;	/* save message pointer for later restoration */
 		pAction->f_pMsg = pMsg;	/* use the new msg (pointer will be restored below) */
diff --git a/plugins/imdiag/imdiag.c b/plugins/imdiag/imdiag.c
index 51f319ca..bfb4a2e5 100644
--- a/plugins/imdiag/imdiag.c
+++ b/plugins/imdiag/imdiag.c
@@ -205,8 +205,7 @@ doInjectMsg(int iNum)
 	datetime.getCurrTime(&stTime, &ttGenTime);
 	/* we now create our own message object and submit it to the queue */
 	CHKiRet(msgConstructWithTime(&pMsg, &stTime, ttGenTime));
-	CHKmalloc(pMsg->pszRawMsg = ustrdup(szMsg));
-	pMsg->iLenRawMsg = ustrlen(szMsg);
+	MsgSetRawMsg(pMsg, (char*) szMsg, ustrlen(szMsg));
 	MsgSetInputName(pMsg, UCHAR_CONSTANT("imdiag"), sizeof("imdiag")-1);
 	MsgSetFlowControlType(pMsg, eFLOWCTL_NO_DELAY);
 	pMsg->msgFlags  = NEEDS_PARSING | PARSE_HOSTNAME;
diff --git a/plugins/imfile/imfile.c b/plugins/imfile/imfile.c
index 86270e2d..bdd222c4 100644
--- a/plugins/imfile/imfile.c
+++ b/plugins/imfile/imfile.c
@@ -97,7 +97,7 @@ static rsRetVal enqLine(fileInfo_t *pInfo, cstr_t *cstrLine)
 	CHKiRet(msgConstruct(&pMsg));
 	MsgSetFlowControlType(pMsg, eFLOWCTL_FULL_DELAY);
 	MsgSetInputName(pMsg, UCHAR_CONSTANT("imfile"), sizeof("imfile")-1);
-	MsgSetRawMsg(pMsg, (char*)rsCStrGetSzStr(cstrLine));
+	MsgSetRawMsg(pMsg, (char*)rsCStrGetSzStr(cstrLine), cstrLen(cstrLine));
 	MsgSetMSG(pMsg, (char*)rsCStrGetSzStr(cstrLine));
 	MsgSetHOSTNAME(pMsg, glbl.GetLocalHostName());
 	MsgSetTAG(pMsg, (char*)pInfo->pszTag);
diff --git a/plugins/imklog/imklog.c b/plugins/imklog/imklog.c
index 420ebbf1..24f15510 100644
--- a/plugins/imklog/imklog.c
+++ b/plugins/imklog/imklog.c
@@ -96,7 +96,7 @@ enqMsg(uchar *msg, uchar* pszTag, int iFacility, int iSeverity)
 	CHKiRet(msgConstruct(&pMsg));
 	MsgSetFlowControlType(pMsg, eFLOWCTL_LIGHT_DELAY);
 	MsgSetInputName(pMsg, UCHAR_CONSTANT("imklog"), sizeof("imklog")-1);
-	MsgSetRawMsg(pMsg, (char*)msg);
+	MsgSetRawMsgWOSize(pMsg, (char*)msg);
 	MsgSetMSG(pMsg, (char*)msg);
 	MsgSetRcvFrom(pMsg, glbl.GetLocalHostName());
 	MsgSetRcvFromIP(pMsg, (uchar*)"127.0.0.1");
diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c
index 97e66e8e..2340aac4 100644
--- a/plugins/imudp/imudp.c
+++ b/plugins/imudp/imudp.c
@@ -241,10 +241,7 @@ processSocket(int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted,
 			}
 			/* we now create our own message object and submit it to the queue */
 			CHKiRet(msgConstructWithTime(&pMsg, &stTime, ttGenTime));
-			/* first trim the buffer to what we have actually received */
-			CHKmalloc(pMsg->pszRawMsg = malloc(sizeof(uchar)* lenRcvBuf));
-			memcpy(pMsg->pszRawMsg, pRcvBuf, lenRcvBuf);
-			pMsg->iLenRawMsg = lenRcvBuf;
+			MsgSetRawMsg(pMsg, (char*)pRcvBuf, lenRcvBuf);
 			MsgSetInputName(pMsg, UCHAR_CONSTANT("imudp"), sizeof("imudp")-1);
 			MsgSetFlowControlType(pMsg, eFLOWCTL_NO_DELAY);
 			pMsg->msgFlags  = NEEDS_PARSING | PARSE_HOSTNAME;
diff --git a/runtime/msg.c b/runtime/msg.c
index d9ff2e73..be62e52f 100644
--- a/runtime/msg.c
+++ b/runtime/msg.c
@@ -510,7 +510,8 @@ CODESTARTobjDestruct(msg)
 	if(currRefCount == 0)
 	{
 		/* DEV Debugging Only! dbgprintf("msgDestruct\t0x%lx, RefCount now 0, doing DESTROY\n", (unsigned long)pThis); */
-		free(pThis->pszRawMsg);
+		if(pThis->pszRawMsg != pThis->szRawMsg)
+			free(pThis->pszRawMsg);
 		free(pThis->pszTAG);
 		free(pThis->pszHOSTNAME);
 		free(pThis->pszInputName);
@@ -876,11 +877,12 @@ char *getMSG(msg_t *pM)
 {
 	if(pM == NULL)
 		return "";
-	else
+	else {
 		if(pM->pszMSG == NULL)
 			return "";
 		else
 			return (char*)pM->pszMSG;
+	}
 }
 
 
@@ -1674,19 +1676,38 @@ void MsgSetMSG(msg_t *pMsg, char* pszMSG)
 		dbgprintf("MsgSetMSG could not allocate memory for pszMSG buffer.");
 }
 
-/* rgerhards 2004-11-11: set RawMsg in msg object
+/* set raw message in message object. Size of message is provided.
+ * rgerhards, 2009-06-16
  */
-void MsgSetRawMsg(msg_t *pMsg, char* pszRawMsg)
+void MsgSetRawMsg(msg_t *pMsg, char* pszRawMsg, size_t lenMsg)
 {
 	assert(pMsg != NULL);
-	if(pMsg->pszRawMsg != NULL)
+	if(pMsg->pszRawMsg != pMsg->szRawMsg)
 		free(pMsg->pszRawMsg);
 
-	pMsg->iLenRawMsg = strlen(pszRawMsg);
-	if((pMsg->pszRawMsg = (uchar*) malloc(pMsg->iLenRawMsg + 1)) != NULL)
-		memcpy(pMsg->pszRawMsg, pszRawMsg, pMsg->iLenRawMsg + 1);
-	else
-		dbgprintf("Could not allocate memory for pszRawMsg buffer.");
+	pMsg->iLenRawMsg = lenMsg;
+	if(pMsg->iLenRawMsg < CONF_RAWMSG_BUFSIZE) {
+		/* small enough: use fixed buffer (faster!) */
+		pMsg->pszRawMsg = pMsg->szRawMsg;
+	} else if((pMsg->pszRawMsg = (uchar*) malloc(pMsg->iLenRawMsg + 1)) == NULL) {
+		/* truncate message, better than completely loosing it... */
+		pMsg->pszRawMsg = pMsg->szRawMsg;
+		pMsg->iLenRawMsg = CONF_RAWMSG_BUFSIZE - 1;
+	}
+
+	memcpy(pMsg->pszRawMsg, pszRawMsg, pMsg->iLenRawMsg);
+	pMsg->pszRawMsg[pMsg->iLenRawMsg] = '\0'; /* this also works with truncation! */
+}
+
+
+/* set raw message in message object. Size of message is not provided. This
+ * function should only be used when it is unavoidable (and over time we should
+ * try to remove it altogether).
+ * rgerhards, 2009-06-16
+ */
+void MsgSetRawMsgWOSize(msg_t *pMsg, char* pszRawMsg)
+{
+	MsgSetRawMsg(pMsg, pszRawMsg, strlen(pszRawMsg));
 }
 
 
@@ -2558,7 +2579,7 @@ rsRetVal MsgSetProperty(msg_t *pThis, var_t *pProp)
  	} else if(isProp("msgFlags")) {
 		pThis->msgFlags = pProp->val.num;
 	} else if(isProp("pszRawMsg")) {
-		MsgSetRawMsg(pThis, (char*) rsCStrGetSzStrNoNULL(pProp->val.pStr));
+		MsgSetRawMsg(pThis, (char*) rsCStrGetSzStrNoNULL(pProp->val.pStr), cstrLen(pProp->val.pStr));
  	/* enable this, if someone actually uses UxTradMsg, delete after some  time has
 	 * passed and nobody complained -- rgerhards, 2009-06-16
 	} else if(isProp("offAfterPRI")) {
diff --git a/runtime/msg.h b/runtime/msg.h
index 703fdd9f..f2701780 100644
--- a/runtime/msg.h
+++ b/runtime/msg.h
@@ -28,6 +28,9 @@
 #ifndef	MSG_H_INCLUDED
 #define	MSG_H_INCLUDED 1
 
+/* some configuration constants */
+#define CONF_RAWMSG_BUFSIZE 101
+
 #include <pthread.h>
 #include "obj.h"
 #include "syslogd-types.h"
@@ -109,6 +112,7 @@ struct msg {
 	int msgFlags;		/* flags associated with this message */
 	ruleset_t *pRuleset;	/* ruleset to be used for processing this message */
 	/* some fixed-size buffers to save malloc()/free() for frequently used fields (from the default templates) */
+	uchar szRawMsg[CONF_RAWMSG_BUFSIZE];	/* most messages are small, and these are stored here (without malloc/free!) */
 	char pszTimestamp3164[16];
 	char pszTimestamp3339[33];
 };
@@ -149,7 +153,8 @@ void MsgAssignHOSTNAME(msg_t *pMsg, char *pBuf);
 void MsgSetHOSTNAME(msg_t *pMsg, uchar* pszHOSTNAME);
 rsRetVal MsgSetAfterPRIOffs(msg_t *pMsg, short offs);
 void MsgSetMSG(msg_t *pMsg, char* pszMSG);
-void MsgSetRawMsg(msg_t *pMsg, char* pszRawMsg);
+void MsgSetRawMsgWOSize(msg_t *pMsg, char* pszRawMsg);
+void MsgSetRawMsg(msg_t *pMsg, char* pszRawMsg, size_t lenMsg);
 void moveHOSTNAMEtoTAG(msg_t *pM);
 char *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe,
                  cstr_t *pCSPropName, unsigned short *pbMustBeFreed);
diff --git a/runtime/parser.c b/runtime/parser.c
index c13edb8f..75cde343 100644
--- a/runtime/parser.c
+++ b/runtime/parser.c
@@ -114,10 +114,8 @@ static inline rsRetVal uncompressMessage(msg_t *pMsg)
 				    "Message ignored.", ret);
 			FINALIZE; /* unconditional exit, nothing left to do... */
 		}
-		free(pMsg->pszRawMsg);
-		pMsg->pszRawMsg = deflateBuf;
-		pMsg->iLenRawMsg = iLenDefBuf;
-		deflateBuf = NULL; /* logically "freed" - caller is now responsible */
+		MsgSetRawMsg(pMsg, (char*)deflateBuf, iLenDefBuf);
+		free(deflateBuf);
 	}
 finalize_it:
 	if(deflateBuf != NULL)
@@ -165,6 +163,8 @@ sanitizeMessage(msg_t *pMsg)
 	size_t iSrc;
 	size_t iDst;
 	size_t iMaxLine;
+	size_t maxDest;
+	uchar szSanBuf[32*1024]; /* buffer used for sanitizing a string */
 
 	assert(pMsg != NULL);
 
@@ -205,70 +205,43 @@ sanitizeMessage(msg_t *pMsg)
 			}
 		}
 	}
-	if(bNeedSanitize == 0) {
-		/* what a shame - we do not have a \0 byte...
-		 * TODO: think about adding it or otherwise be able to use it...
-		 */
-		uchar *pRaw;
-		CHKmalloc(pRaw = realloc(pMsg->pszRawMsg, pMsg->iLenRawMsg + 1));
-		pRaw[pMsg->iLenRawMsg] = '\0';
-		pMsg->pszRawMsg = pRaw;
+
+	if(!bNeedSanitize)
 		FINALIZE;
-	}
 
 	/* now copy over the message and sanitize it */
-	/* TODO: can we get cheaper memory alloc? {alloca()?}*/
 	iMaxLine = glbl.GetMaxLine();
-	CHKmalloc(pDst = malloc(sizeof(uchar) * (iMaxLine + 1)));
+	maxDest = lenMsg * 4; /* message can grow at most four-fold */
+	if(maxDest > iMaxLine)
+		maxDest = iMaxLine;	/* but not more than the max size! */
+	if(maxDest < sizeof(szSanBuf))
+		pDst = szSanBuf;
+	else 
+		CHKmalloc(pDst = malloc(sizeof(uchar) * (iMaxLine + 1)));
 	iSrc = iDst = 0;
-	while(iSrc < lenMsg && iDst < iMaxLine) {
+	while(iSrc < lenMsg && iDst < maxDest - 3) { /* leave some space if last char must be escaped */
 		if(pszMsg[iSrc] == '\0') { /* guard against \0 characters... */
-			/* changed to the sequence (somewhat) proposed in
-			 * draft-ietf-syslog-protocol-19. rgerhards, 2006-11-30
-			 */
-			if(iDst + 3 < iMaxLine) { /* do we have space? */
-				pDst[iDst++] =  cCCEscapeChar;
-				pDst[iDst++] = '0';
-				pDst[iDst++] = '0';
-				pDst[iDst++] = '0';
-			} /* if we do not have space, we simply ignore the '\0'... */
-			  /* log an error? Very questionable... rgerhards, 2006-11-30 */
-			  /* decided: we do not log an error, it won't help... rger, 2007-06-21 */
-		} else if(bEscapeCCOnRcv && iscntrl((int) pszMsg[iSrc])) {
+		} else if(iscntrl((int) pszMsg[iSrc])) {
+			if(pszMsg[iSrc] == '\0' || bEscapeCCOnRcv) {
 			/* we are configured to escape control characters. Please note
 			 * that this most probably break non-western character sets like
 			 * Japanese, Korean or Chinese. rgerhards, 2007-07-17
-			 * Note: sysklogd logs octal values only for DEL and CCs above 127.
-			 * For others, it logs ^n where n is the control char converted to an
-			 * alphabet character. We like consistency and thus escape it to octal
-			 * in all cases. If someone complains, we may change the mode. At least
-			 * we known now what's going on.
-			 * rgerhards, 2007-07-17
 			 */
-			if(iDst + 3 < iMaxLine) { /* do we have space? */
-				pDst[iDst++] = cCCEscapeChar;
-				pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0300) >> 6);
-				pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0070) >> 3);
-				pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0007));
-			} /* again, if we do not have space, we ignore the char - see comment at '\0' */
+			pDst[iDst++] = cCCEscapeChar;
+			pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0300) >> 6);
+			pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0070) >> 3);
+			pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0007));
+			}
 		} else {
 			pDst[iDst++] = pszMsg[iSrc];
 		}
 		++iSrc;
 	}
-	pDst[iDst] = '\0'; /* space *is* reserved for this! */
-
-	/* we have a sanitized string. Let's save it now */
-	free(pMsg->pszRawMsg);
-	if((pMsg->pszRawMsg = malloc((iDst+1) * sizeof(uchar))) == NULL) {
-		/* when we get no new buffer, we use what we already have ;) */
-		pMsg->pszRawMsg = pDst;
-	} else {
-		/* trim buffer */
-		memcpy(pMsg->pszRawMsg, pDst, iDst+1);
-		free(pDst); /* too big! */
-		pMsg->iLenRawMsg = iDst;
-	}
+
+	MsgSetRawMsg(pMsg, (char*)pDst, iDst); /* save sanitized string */
+
+	if(pDst != szSanBuf)
+		free(pDst);
 
 finalize_it:
 	RETiRet;
diff --git a/tcps_sess.c b/tcps_sess.c
index 9353571c..e0bec949 100644
--- a/tcps_sess.c
+++ b/tcps_sess.c
@@ -234,10 +234,8 @@ defaultDoSubmitMessage(tcps_sess_t *pThis, struct syslogTime *stTime, time_t ttG
 
 	/* we now create our own message object and submit it to the queue */
 	CHKiRet(msgConstructWithTime(&pMsg, stTime, ttGenTime));
-	/* first trim the buffer to what we have actually received */
-	CHKmalloc(pMsg->pszRawMsg = malloc(sizeof(uchar) * pThis->iMsg));
-	memcpy(pMsg->pszRawMsg, pThis->pMsg, pThis->iMsg);
-	pMsg->iLenRawMsg = pThis->iMsg;
+dbgprintf("defaultDoSubmit, iMsg %d\n", pThis->iMsg);
+	MsgSetRawMsg(pMsg, (char*)pThis->pMsg, pThis->iMsg);
 	MsgSetInputName(pMsg, pThis->pLstnInfo->pszInputName, pThis->pLstnInfo->lenInputName);
 	MsgSetFlowControlType(pMsg, eFLOWCTL_LIGHT_DELAY);
 	pMsg->msgFlags  = NEEDS_PARSING | PARSE_HOSTNAME;
diff --git a/tests/nettester.c b/tests/nettester.c
index b7b3f892..e5e6278c 100644
--- a/tests/nettester.c
+++ b/tests/nettester.c
@@ -191,7 +191,7 @@ int openPipe(char *configFile, pid_t *pid, int *pfd)
 	char *newenviron[] = { NULL };
 	/* debug aide...
 	char *newenviron[] = { "RSYSLOG_DEBUG=debug nostdout",
-				"RSYSLOG_DEBUGLOG=tmp", NULL };
+				"RSYSLOG_DEBUGLOG=log", NULL };
 	*/
 
 
diff --git a/tests/parsertest.sh b/tests/parsertest.sh
index afdb9469..8e04b95e 100755
--- a/tests/parsertest.sh
+++ b/tests/parsertest.sh
@@ -1,13 +1,13 @@
 echo test parsertest via udp
 $srcdir/killrsyslog.sh # kill rsyslogd if it runs for some reason
 
-./nettester -tparse1 -iudp
+echo test parsertest via tcp
+./nettester -tparse1 -itcp
 if [ "$?" -ne "0" ]; then
   exit 1
 fi
 
-echo test parsertest via tcp
-./nettester -tparse1 -itcp
+./nettester -tparse1 -iudp
 if [ "$?" -ne "0" ]; then
   exit 1
 fi
diff --git a/tools/syslogd.c b/tools/syslogd.c
index 034111f5..91918d96 100644
--- a/tools/syslogd.c
+++ b/tools/syslogd.c
@@ -582,7 +582,7 @@ static inline rsRetVal printline(uchar *hname, uchar *hnameIP, uchar *msg, int f
 	if(pszInputName != NULL)
 		MsgSetInputName(pMsg, pszInputName, ustrlen(pszInputName));
 	MsgSetFlowControlType(pMsg, flowCtlType);
-	MsgSetRawMsg(pMsg, (char*)msg);
+	MsgSetRawMsgWOSize(pMsg, (char*)msg);
 	
 	/* test for special codes */
 	pri = DEFUPRI;
@@ -882,7 +882,7 @@ logmsgInternal(int iErr, int pri, uchar *msg, int flags)
 
 	CHKiRet(msgConstruct(&pMsg));
 	MsgSetInputName(pMsg, UCHAR_CONSTANT("rsyslogd"), sizeof("rsyslogd")-1);
-	MsgSetRawMsg(pMsg, (char*)msg);
+	MsgSetRawMsgWOSize(pMsg, (char*)msg);
 	MsgSetHOSTNAME(pMsg, glbl.GetLocalHostName());
 	MsgSetRcvFrom(pMsg, glbl.GetLocalHostName());
 	MsgSetRcvFromIP(pMsg, UCHAR_CONSTANT("127.0.0.1"));