- added CODE_STD_FINALIZERparseSelectorAct to module-generation macros

- restructered rsyslogd startup - moved startWorker() to a more appropriate
    place
- updated ommysql.c to fully support suspension/resumption by rule engine

9 files changed, 137 insertions, 258 deletions

diff --git a/module-template.h b/module-template.h
index 2ef4a174..54f18cfa 100644
--- a/module-template.h
+++ b/module-template.h
@@ -217,12 +217,11 @@ static rsRetVal parseSelectorAct(uchar **pp, void **ppModData, omodStringRequest
 	p = *pp;
 
 #define CODE_STD_STRING_REQUESTparseSelectorAct(NumStrReqEntries) \
-	if((iRet = OMSRconstruct(ppOMSR, NumStrReqEntries)) != RS_RET_OK)\
-		goto do_abort;\
+	CHKiRet(OMSRconstruct(ppOMSR, NumStrReqEntries));
 
-#define ENDparseSelectorAct \
-do_abort:\
-	if(iRet == RS_RET_OK) {\
+#define CODE_STD_FINALIZERparseSelectorAct \
+finalize_it:\
+	if(iRet == RS_RET_OK || iRet == RS_RET_SUSPENDED) {\
 		*ppModData = pData;\
 		*pp = p;\
 	} else {\
@@ -230,7 +229,9 @@ do_abort:\
 		if(*ppOMSR != NULL)\
 			OMSRdestruct(*ppOMSR);\
 			*ppOMSR = NULL;\
-		}\
+		}
+
+#define ENDparseSelectorAct \
 	return iRet;\
 }
 
diff --git a/omdiscard.c b/omdiscard.c
index b6ba3a2f..75eb953c 100644
--- a/omdiscard.c
+++ b/omdiscard.c
@@ -92,6 +92,7 @@ CODE_STD_STRING_REQUESTparseSelectorAct(0)
 	} else {
 		iRet = RS_RET_CONFLINE_UNPROCESSED;
 	}
+CODE_STD_FINALIZERparseSelectorAct
 ENDparseSelectorAct
 
 
diff --git a/omfile.c b/omfile.c
index ce42997c..cd19aad0 100644
--- a/omfile.c
+++ b/omfile.c
@@ -761,6 +761,7 @@ CODESTARTparseSelectorAct
 		iRet = RS_RET_CONFLINE_UNPROCESSED;
 		break;
 	}
+CODE_STD_FINALIZERparseSelectorAct
 ENDparseSelectorAct
 
 
diff --git a/omfwd.c b/omfwd.c
index 089749bf..c1172b0c 100644
--- a/omfwd.c
+++ b/omfwd.c
@@ -865,6 +865,7 @@ CODE_STD_STRING_REQUESTparseSelectorAct(1)
 	/* TODO: do we need to call freeInstance if we failed - this is a general question for
 	 * all output modules. I'll address it lates as the interface evolves. rgerhards, 2007-07-25
 	 */
+CODE_STD_FINALIZERparseSelectorAct
 ENDparseSelectorAct
 
 
diff --git a/ommysql.c b/ommysql.c
index ea0012b1..573e72ec 100644
--- a/ommysql.c
+++ b/ommysql.c
@@ -5,10 +5,6 @@
  *       works!
  *
  * File begun on 2007-07-20 by RGerhards (extracted from syslogd.c)
- * This file is under development and has not yet arrived at being fully
- * self-contained and a real object. So far, it is mostly an excerpt
- * of the "old" message code without any modifications. However, it
- * helps to have things at the right place one we go to the meat of it.
  *
  * Copyright 2007 Rainer Gerhards and Adiscon GmbH.
  *
@@ -58,10 +54,6 @@ typedef struct _instanceData {
 	char	f_dbname[_DB_MAXDBLEN+1];	/* DB name */
 	char	f_dbuid[_DB_MAXUNAMELEN+1];	/* DB user */
 	char	f_dbpwd[_DB_MAXPWDLEN+1];	/* DB user's password */
-	time_t	f_timeResumeOnError;		/* 0 if no error is present,	
-						   otherwise is it set to the
-						   time a retrail should be attampt */
-	int	f_iLastDBErrNo;			/* Last db error number. 0 = no error */
 } instanceData;
 
 
@@ -84,16 +76,17 @@ ENDisCompatibleWithFeature
 static void closeMySQL(instanceData *pData)
 {
 	assert(pData != NULL);
+
 	dprintf("in closeMySQL\n");
-	if(pData->f_hmysql != NULL)	/* just to be on the safe side... */
+	if(pData->f_hmysql != NULL) {	/* just to be on the safe side... */
 		mysql_close(pData->f_hmysql);	
+		pData->f_hmysql = NULL;
+	}
 }
 
 BEGINfreeInstance
 CODESTARTfreeInstance
-#	ifdef	WITH_DB
 	closeMySQL(pData);
-#	endif
 ENDfreeInstance
 
 
@@ -118,210 +111,93 @@ CODESTARTgetWriteFDForSelect
 ENDgetWriteFDForSelect
 
 
-static rsRetVal reInitMySQL(instanceData *);
-
-
-/**
- * DBErrorHandler
- *
- * Call this function if an db error apears. It will initiate 
- * the "delay" handling which stopped the db logging for some 
- * time.  
- *
- * We now check if we have a valid MySQL handle. If not, we simply
+/* log a database error with descriptive message.
+ * We check if we have a valid MySQL handle. If not, we simply
  * report an error, but can not be specific. RGerhards, 2007-01-30
  */
-static void DBErrorHandler(instanceData *pData)
+static void reportDBError(instanceData *pData)
 {
 	char errMsg[512];
 
 	assert(pData != NULL);
 
-	/* TODO:
-	 * NO DB connection -> Can not log to DB
-	 * -------------------- 
-	 * Case 1: db server unavailable
-	 * We can check after a specified time interval if the server is up.
-	 * Also a reason can be a down DNS service.
-	 * Case 2: uid, pwd or dbname are incorrect
-	 * If this is a fault in the syslog.conf we have no chance to recover. But
-	 * if it is a problem of the DB we can make a retry after some time. Possible
-	 * are that the admin has not already set up the database table. Or he has not
-	 * created the database user yet. 
-	 * Case 3: unkown error
-	 * If we get an unkowon error here, we should in any case try to recover after
-	 * a specified time interval.
-	 *
-	 * Insert failed -> Can not log to DB
-	 * -------------------- 
-	 * If the insert fails it is never a good idea to give up. Only an
-	 * invalid sql sturcture (wrong template) force us to disable db
-	 * logging. 
-	 *
-	 * Think about different "delay" for different errors!
-	 */
+	/* output log message */
+	errno = 0;
 	if(pData->f_hmysql == NULL) {
-		logerror("unknown DB error occured - called error handler with NULL MySQL handle.");
+		logerror("unknown DB error occured - could not obtain MySQL handle");
 	} else { /* we can ask mysql for the error description... */
-		errno = 0;
-		snprintf(errMsg, sizeof(errMsg)/sizeof(char),
-			"db error (%d): %s\n", mysql_errno(pData->f_hmysql),
+		snprintf(errMsg, sizeof(errMsg)/sizeof(char), "db error (%d): %s\n", mysql_errno(pData->f_hmysql),
 			mysql_error(pData->f_hmysql));
-		pData->f_iLastDBErrNo = mysql_errno(pData->f_hmysql);
 		logerror(errMsg);
 	}
 		
-	/* Enable "delay" */
-	pData->f_timeResumeOnError = time(&pData->f_timeResumeOnError) + _DB_DELAYTIMEONERROR ;
+	return;
 }
 
 
-/**
- * checkDBErrorState
- *
- * Check if we can go on with database logging or if we should wait
- * a little bit longer. It also check if the DB hanlde is still valid. 
- * If it is necessary, it takes action to reinitiate the db connection.
- *
- * \ret int		Returns 0 if successful (no error)
- */ 
-rsRetVal checkDBErrorState(instanceData *pData)
-{
-	time_t now;
-	assert(pData != NULL);
-	/* dprintf("in checkDBErrorState, timeResumeOnError: %d\n", pData->f_timeResumeOnError); */
-
-	/* If timeResumeOnError == 0 no error occured, 
-	   we can return with 0 (no error) */
-	if (pData->f_timeResumeOnError == 0)
-		return RS_RET_OK;
-	
-	(void) time(&now);
-	/* Now we know an error occured. We check timeResumeOnError
-	   if we can process. If we have not reach the resume time
-	   yet, we return an error status. */  
-	if (pData->f_timeResumeOnError > now)
-	{
-		/* dprintf("Wait time is not over yet.\n"); */
-		return RS_RET_ERR;
-	}
-	 	
-	/* Ok, we can try to resume the database logging. First
-	   we have to reset the status (timeResumeOnError) and
-	   the last error no. */
-	/* To improve this code it would be better to check 
-	   if we really need to reInit the db connection. If 
-	   only the insert failed and the db conncetcion is
-	   still valid, we need no reInit. 
-	   Of course, if an unkown error appeared, we should
-	   reInit. */
-	 /* rgerhards 2004-12-08: I think it is pretty unlikely
-	  * that we can re-use a connection after the error. So I guess
-	  * the connection must be closed and re-opened in all cases
-	  * (as it is done currently). When we come back to optimize
-	  * this code, we should anyhow see if there are cases where
-	  * we could keep it open. I just doubt this won't be the case.
-	  * I added this comment (and did not remove Michaels) just so
-	  * that we all know what we are looking for.
-	  */
-	pData->f_timeResumeOnError = 0;
-	pData->f_iLastDBErrNo = 0; 
-	return reInitMySQL(pData);
-}
-
-/*
- * The following function is responsible for initializing a
+/* The following function is responsible for initializing a
  * MySQL connection.
  * Initially added 2004-10-28 mmeckelein
  */
 static rsRetVal initMySQL(instanceData *pData)
 {
-	int iCounter = 0;
 	DEFiRet;
+
 	assert(pData != NULL);
+	assert(pData->f_hmysql == NULL);
 
-	if((iRet = checkDBErrorState(pData)) != RS_RET_OK)
-		return iRet;
-	
 	pData->f_hmysql = mysql_init(NULL);
 	if(pData->f_hmysql == NULL) {
-		logerror("can not initialize MySQL handle - ignoring this action");
-		/* The next statement  causes a redundant message, but it is the
-		 * best thing we can do in this situation. -- rgerhards, 2007-01-30
-	     	 */
-		 iRet = RS_RET_DISABLE_ACTION;
+		logerror("can not initialize MySQL handle");
+		iRet = RS_RET_SUSPENDED;
 	} else { /* we could get the handle, now on with work... */
-		do {
-			/* Connect to database */
-			if (!mysql_real_connect(pData->f_hmysql, pData->f_dbsrv, pData->f_dbuid,
-			                        pData->f_dbpwd, pData->f_dbname, 0, NULL, 0)) {
-				/* if also the second attempt failed
-				   we call the error handler */
-				if(iCounter)
-					DBErrorHandler(pData);
-			} else {
-				pData->f_timeResumeOnError = 0; /* We have a working db connection */
-				dprintf("connected successfully to db\n");
-			}
-			iCounter++;
-		} while (mysql_errno(pData->f_hmysql) && iCounter<2);
+		/* Connect to database */
+		if(mysql_real_connect(pData->f_hmysql, pData->f_dbsrv, pData->f_dbuid,
+				      pData->f_dbpwd, pData->f_dbname, 0, NULL, 0) == NULL) {
+			reportDBError(pData);
+			closeMySQL(pData); /* ignore any error we may get */
+			iRet = RS_RET_SUSPENDED;
+		}
 	}
-	return iRet;
-}
 
-/*
- * Reconnect a MySQL connection.
- * Initially added 2004-12-02
- */
-static rsRetVal reInitMySQL(instanceData *pData)
-{
-	assert(pData != NULL);
-
-	dprintf("reInitMySQL\n");
-	closeMySQL(pData); /* close the current handle */
-	return initMySQL(pData); /* new connection */   
+	return iRet;
 }
 
 
-
 /* The following function writes the current log entry
  * to an established MySQL session.
  * Initially added 2004-10-28 mmeckelein
  */
 rsRetVal writeMySQL(uchar *psz, instanceData *pData)
 {
-	int iCounter=0;
 	DEFiRet;
-	assert(pData != NULL);
 
-	if((iRet = checkDBErrorState(pData)) != RS_RET_OK)
-		return iRet;
+	assert(psz != NULL);
+	assert(pData != NULL);
 
-	/* Now we are trying to insert the data. 
-	 *
-	 * If the first attampt fails we simply try a second one. If that
-	 * fails too, we discard the message and enable "delay" error handling.
-	 */
-	do {
-		/* query */
-		if(mysql_query(pData->f_hmysql, (char*)psz)) {
-			/* if the second attempt fails
-			   we call the error handler */
-			if(iCounter)
-				DBErrorHandler(pData);
-		}
-		else {
-			/* dprintf("db insert sucessfull\n"); */
+	/* try insert */
+	if(mysql_query(pData->f_hmysql, (char*)psz)) {
+		/* error occured, try to re-init connection and retry */
+		closeMySQL(pData); /* close the current handle */
+		CHKiRet(initMySQL(pData)); /* try to re-open */
+		if(mysql_query(pData->f_hmysql, (char*)psz)) { /* re-try insert */
+			/* we failed, giving up for now */
+			reportDBError(pData);
+			closeMySQL(pData); /* free ressources */
+			ABORT_FINALIZE(RS_RET_SUSPENDED);
 		}
-		iCounter++;
-	} while (mysql_errno(pData->f_hmysql) && iCounter<2);
+	}
+
+finalize_it:
 	return iRet;
 }
 
 
 BEGINtryResume
 CODESTARTtryResume
-dprintf("tryResume returns %d\n", iRet);
+	if(pData->f_hmysql == NULL) {
+		iRet = initMySQL(pData);
+	}
 ENDtryResume
 
 BEGINdoAction
@@ -335,80 +211,73 @@ BEGINparseSelectorAct
 	int iMySQLPropErr = 0;
 CODESTARTparseSelectorAct
 CODE_STD_STRING_REQUESTparseSelectorAct(1)
-	/* yes, the if below is redundant, but I need it now. Will go away as
-	 * the code further changes.  -- rgerhards, 2007-07-25
-	 */
-	if(*p == '>') {
-		if((iRet = createInstance(&pData)) != RS_RET_OK)
-			return iRet;
+	/* first check if this config line is actually for us */
+	if(*p != '>') {
+		ABORT_FINALIZE(RS_RET_CONFLINE_UNPROCESSED);
 	}
 
-	switch (*p)
-	{
-	case '>':       /* rger 2004-10-28: added support for MySQL
-			 * >server,dbname,userid,password
-			 * rgerhards 2005-08-12: changed rsyslogd so that
-			 * if no DB is selected and > is used, an error
-			 * message is logged.
-			 */
-		if(bModMySQLLoaded == 0)
-			logerror("To enable MySQL logging, a \"$ModLoad MySQL\" must be done - accepted for "
-		        	 "the time being, but will fail in future releases.");
+	/* ok, if we reach this point, we have something for us */
+	if((iRet = createInstance(&pData)) != RS_RET_OK)
+		return iRet;
+
+	p++; /* eat '>' '*/
+
+	/* rger 2004-10-28: added support for MySQL
+	 * >server,dbname,userid,password
+	 * rgerhards 2005-08-12: changed rsyslogd so that
+	 * if no DB is selected and > is used, an error
+	 * message is logged.
+	 */
+	if(bModMySQLLoaded == 0)
+		logerror("To enable MySQL logging, a \"$ModLoad MySQL\" must be done - accepted for "
+			 "the time being, but will fail in future releases.");
 #ifndef	WITH_DB
-		iRet = RS_RET_ERROR; /* this goes away anyhow, so it's not worth putting much effort in the return code */
-		errno = 0;
-		logerror("write to database action in config file, but rsyslogd compiled without "
-		         "database functionality - ignored");
+	iRet = RS_RET_ERROR; /* this goes away anyhow, so it's not worth putting much effort in the return code */
+	errno = 0;
+	logerror("write to database action in config file, but rsyslogd compiled without "
+		 "database functionality - ignored");
 #else /* WITH_DB defined! */
-		p++;
-		
-		/* Now we read the MySQL connection properties 
-		 * and verify that the properties are valid.
-		 */
-		if(getSubString(&p, pData->f_dbsrv, MAXHOSTNAMELEN+1, ','))
-			iMySQLPropErr++;
-		if(*pData->f_dbsrv == '\0')
-			iMySQLPropErr++;
-		if(getSubString(&p, pData->f_dbname, _DB_MAXDBLEN+1, ','))
-			iMySQLPropErr++;
-		if(*pData->f_dbname == '\0')
-			iMySQLPropErr++;
-		if(getSubString(&p, pData->f_dbuid, _DB_MAXUNAMELEN+1, ','))
-			iMySQLPropErr++;
-		if(*pData->f_dbuid == '\0')
-			iMySQLPropErr++;
-		if(getSubString(&p, pData->f_dbpwd, _DB_MAXPWDLEN+1, ';'))
-			iMySQLPropErr++;
-		/* now check for template
-		 * We specify that the SQL option must be present in the template.
-		 * This is for your own protection (prevent sql injection).
-		 */
-		if(*p != ';')
-			--p;	/* TODO: the whole parsing of the MySQL module needs to be re-thought - but this here
-				 *       is clean enough for the time being -- rgerhards, 2007-07-30
-			         */
-		if((iRet = cflineParseTemplateName(&p, *ppOMSR, 0, OMSR_RQD_TPL_OPT_SQL, (uchar*) " StdDBFmt"))
-		   != RS_RET_OK)
-			return iRet;
-		
-		/* If we detect invalid properties, we disable logging, 
-		 * because right properties are vital at this place.  
-		 * Retries make no sense. 
-		 */
-		if (iMySQLPropErr) { 
-			iRet = RS_RET_ERR; /* re-vist error code when working on this module */
-			dprintf("Trouble with MySQL conncetion properties.\n"
-				"MySQL logging disabled.\n");
-			break;
-		} else {
-			initMySQL(pData);
-		}
-#endif	/* #ifdef WITH_DB */
-		break;
-	default:
-		iRet = RS_RET_CONFLINE_UNPROCESSED;
-		break;
+	
+	/* Now we read the MySQL connection properties 
+	 * and verify that the properties are valid.
+	 */
+	if(getSubString(&p, pData->f_dbsrv, MAXHOSTNAMELEN+1, ','))
+		iMySQLPropErr++;
+	if(*pData->f_dbsrv == '\0')
+		iMySQLPropErr++;
+	if(getSubString(&p, pData->f_dbname, _DB_MAXDBLEN+1, ','))
+		iMySQLPropErr++;
+	if(*pData->f_dbname == '\0')
+		iMySQLPropErr++;
+	if(getSubString(&p, pData->f_dbuid, _DB_MAXUNAMELEN+1, ','))
+		iMySQLPropErr++;
+	if(*pData->f_dbuid == '\0')
+		iMySQLPropErr++;
+	if(getSubString(&p, pData->f_dbpwd, _DB_MAXPWDLEN+1, ';'))
+		iMySQLPropErr++;
+	/* now check for template
+	 * We specify that the SQL option must be present in the template.
+	 * This is for your own protection (prevent sql injection).
+	 */
+	if(*p != ';')
+		--p;	/* TODO: the whole parsing of the MySQL module needs to be re-thought - but this here
+			 *       is clean enough for the time being -- rgerhards, 2007-07-30
+			 */
+	CHKiRet(cflineParseTemplateName(&p, *ppOMSR, 0, OMSR_RQD_TPL_OPT_SQL, (uchar*) " StdDBFmt"));
+	
+	/* If we detect invalid properties, we disable logging, 
+	 * because right properties are vital at this place.  
+	 * Retries make no sense. 
+	 */
+	if (iMySQLPropErr) { 
+		logerror("Trouble with MySQL connection properties. -MySQL logging disabled");
+		ABORT_FINALIZE(RS_RET_INVALID_PARAMS);
+	} else {
+		CHKiRet(initMySQL(pData));
 	}
+#endif	/* #ifdef WITH_DB */
+
+CODE_STD_FINALIZERparseSelectorAct
 ENDparseSelectorAct
 
 
diff --git a/omshell.c b/omshell.c
index f2826a75..bc4ea56a 100644
--- a/omshell.c
+++ b/omshell.c
@@ -115,6 +115,7 @@ CODE_STD_STRING_REQUESTparseSelectorAct(1)
 		break;
 	}
 
+CODE_STD_FINALIZERparseSelectorAct
 ENDparseSelectorAct
 
 
diff --git a/omusrmsg.c b/omusrmsg.c
index 7fe56dde..d63f957a 100644
--- a/omusrmsg.c
+++ b/omusrmsg.c
@@ -303,6 +303,7 @@ CODE_STD_STRING_REQUESTparseSelectorAct(1)
 		    != RS_RET_OK)
 			return iRet;
 	}
+CODE_STD_FINALIZERparseSelectorAct
 ENDparseSelectorAct
 
 
diff --git a/rsyslog.h b/rsyslog.h
index c4165bac..e0e8d1ea 100644
--- a/rsyslog.h
+++ b/rsyslog.h
@@ -67,6 +67,7 @@ enum rsRetVal_				/** return value. All methods return this if not specified oth
 	RS_RET_FOPEN_FAILURE = -2013,	/**< failure during fopen, for example file not found - see errno */
 	RS_RET_END_OF_LINKEDLIST = -2014,	/**< end of linked list, not an error, but a status */
 	RS_RET_CHAIN_NOT_PERMITTED = -2015, /**< chaining (e.g. of config command handlers) not permitted */
+	RS_RET_INVALID_PARAMS = -2016,/**< supplied parameters are invalid */
 	RS_RET_OK = 0			/**< operation successful */
 };
 typedef enum rsRetVal_ rsRetVal; /**< friendly type for global return value */
diff --git a/syslogd.c b/syslogd.c
index 1888d47d..7ec50134 100644
--- a/syslogd.c
+++ b/syslogd.c
@@ -1992,7 +1992,7 @@ static rsRetVal actionDbgPrint(action_t *pThis)
 		printf(" next retry: %u, number retries: %d", (unsigned) pThis->ttResumeRtry, pThis->iNbrResRtry);
 	}
 	printf("\n");
-	printf("\tDisabled: %d\n", pThis->bEnabled);
+	printf("\tDisabled: %d\n", !pThis->bEnabled);
 	printf("\tExec only when previous is suspended: %d\n", pThis->bExecWhenPrevSusp);
 	printf("\n");
 
@@ -4269,7 +4269,7 @@ finalize_it:
 /* INIT -- Initialize syslogd from configuration table
  * init() is called at initial startup AND each time syslogd is HUPed
  */
-static void init()
+static void init(void)
 {
 	DEFiRet;
 	register int i;
@@ -4428,6 +4428,10 @@ static void init()
 	}
 #endif
 
+#	ifdef USE_PTHREADS
+		startWorker();
+#	endif
+
 	Initialized = 1;
 
 	if(Debug) {
@@ -5640,9 +5644,6 @@ static void mainloop(void)
 			dprintf("\nReceived SIGHUP, reloading rsyslogd.\n");
 			/* worker thread is stopped as part of init() */
 			init();
-#			ifdef USE_PTHREADS
-				startWorker();
-#			endif
 			restart = 0;
 			continue;
 		}
@@ -5924,12 +5925,25 @@ int main(int argc, char **argv)
 		funix[i]  = -1;
 	}
 
+	/* doing some core initializations */
+
+#ifdef	USE_PTHREADS
+	/* create message queue */
+	pMsgQueue = queueInit();
+	if(pMsgQueue == NULL) {
+		errno = 0;
+		logerror("error: could not create message queue - running single-threaded!\n");
+	}
+#endif
+
 	if((iRet = loadBuildInModules()) != RS_RET_OK) {
 		fprintf(stderr, "fatal error: could not activate built-in modules. Error code %d.\n",
 			iRet);
 		exit(1); /* "good" exit, leaving at init for fatal error */
 	}
 
+	/* END core initializations */
+
 	while ((ch = getopt(argc, argv, "46Aa:dehi:f:l:m:nop:r::s:t:u:vwx")) != EOF) {
 		switch((char)ch) {
                 case '4':
@@ -6192,18 +6206,7 @@ int main(int argc, char **argv)
 	 * do the init() and then restart things.
 	 * rgerhards, 2005-10-24
 	 */
-#ifdef	USE_PTHREADS
-	/* create message queue */
-	pMsgQueue = queueInit();
-	if(pMsgQueue == NULL) {
-		errno = 0;
-		logerror("error: could not create message queue - running single-threaded!\n");
-	} else { /* start up worker thread */
-		startWorker();
-	}
-#endif
 
-	/* --------------------- Main loop begins here. ----------------------------------------- */
 	mainloop();
 	die(bFinished);
 	return 0;