- moved umask & file/dir creation mode parsing to cfsysline.c

- added macro for easy and consistent check of iRet return value

5 files changed, 119 insertions, 84 deletions

diff --git a/ChangeLog b/ChangeLog
index 29eaed88..3c835a8d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,6 @@
 ---------------------------------------------------------------------------
 Version 1.17.6 (rgerhards), 2007-07-3?
+- continued to work on output module modularization
 - fixed bug in OMSRcreate() - always returned SR_RET_OK
 - fixed a bug that caused ommysql to always complain about missing
   templates
@@ -13,7 +14,7 @@ Version 1.17.6 (rgerhards), 2007-07-3?
   hair to find the bug...
 ---------------------------------------------------------------------------
 Version 1.17.5 (rgerhards), 2007-07-30
-- continued to work on modularization
+- continued to work on output module modularization
 - fixed a missing file bug - thanks to Andrea Montanari for reporting
   this problem
 - fixed a problem with shutting down the worker thread and freeing the
@@ -22,14 +23,14 @@ Version 1.17.5 (rgerhards), 2007-07-30
   destroyed.
 ---------------------------------------------------------------------------
 Version 1.17.4 (rgerhards), 2007-07-27
-- continued to work on modularization
+- continued to work on output module modularization
 - fixed a situation where rsyslogd could create zombie processes
   thanks to mildew for the patch
 - applied patch from Michel Samia to fix compilation when NOT
   compiled for pthreads
 ---------------------------------------------------------------------------
 Version 1.17.3 (rgerhards), 2007-07-25
-- continued working on modularization
+- continued working on output module modularization
 - fixed a bug that caused rsyslogd to segfault on exit (and
   probably also on HUP), when there was an unsent message in a selector
   that required forwarding and the dns lookup failed for that selector
diff --git a/cfsysline.c b/cfsysline.c
index 8fb66d89..aa0866b7 100644
--- a/cfsysline.c
+++ b/cfsysline.c
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <assert.h>
 #include <string.h>
+#include <errno.h>
 #include <pwd.h>
 #include <grp.h>
 
@@ -42,6 +43,93 @@ cslCmd_t *pCmdListLast = NULL;
 
 /* --------------- START functions for handling canned syntaxes --------------- */
 
+/* Parse and interpet a $FileCreateMode and $umask line. This function
+ * pulls the creation mode and, if successful, stores it
+ * into the global variable so that the rest of rsyslogd
+ * opens files with that mode. Any previous value will be
+ * overwritten.
+ * HINT: if we store the creation mode in selector_t, we
+ * can even specify multiple modes simply be virtue of
+ * being placed in the right section of rsyslog.conf
+ * rgerhards, 2007-07-4 (happy independence day to my US friends!)
+ * Parameter **pp has a pointer to the current config line.
+ * On exit, it will be updated to the processed position.
+ */
+rsRetVal doFileCreateMode(uchar **pp, rsRetVal (*pSetHdlr)(void*, uid_t), void *pVal)
+{
+	uchar *p;
+	rsRetVal iRet = RS_RET_OK;
+	uchar errMsg[128];	/* for dynamic error messages */
+	int iVal;	
+
+	assert(pp != NULL);
+	assert(*pp != NULL);
+	
+	skipWhiteSpace(pp); /* skip over any whitespace */
+	p = *pp;
+
+	/* for now, we parse and accept only octal numbers
+	 * Sequence of tests is important, we are using boolean shortcuts
+	 * to avoid addressing invalid memory!
+	 */
+	if(!(   (*p == '0')
+	     && (*(p+1) && *(p+1) >= '0' && *(p+1) <= '7')
+	     && (*(p+2) && *(p+2) >= '0' && *(p+2) <= '7')
+	     && (*(p+3) && *(p+3) >= '0' && *(p+3) <= '7')  )  ) {
+		snprintf((char*) errMsg, sizeof(errMsg)/sizeof(uchar),
+		         "value must be octal (e.g 0644).");
+		errno = 0;
+		logerror((char*) errMsg);
+		iRet = RS_RET_INVALID_VALUE;
+		goto finalize_it;
+	}
+
+	/*  we reach this code only if the octal number is ok - so we can now
+	 *  compute the value.
+	 */
+	iVal  = (*(p+1)-'0') * 64 + (*(p+2)-'0') * 8 + (*(p+3)-'0');
+
+	if(pSetHdlr == NULL) {
+		/* we should set value directly to var */
+		*((int*)pVal) = iVal;
+	} else {
+		/* we set value via a set function */
+		CHKiRet(pSetHdlr(pVal, iVal));
+	}
+
+#if 0
+	switch(eDir) {
+		case DIR_DIRCREATEMODE:
+			fDirCreateMode = iMode;
+			dprintf("DirCreateMode set to 0%o.\n", iMode);
+			break;
+		case DIR_FILECREATEMODE:
+			fCreateMode = iMode;
+			dprintf("FileCreateMode set to 0%o.\n", iMode);
+			break;
+		case DIR_UMASK:
+			umask(iMode);
+			dprintf("umask set to 0%3.3o.\n", iMode);
+			break;
+		default:/* we do this to avoid compiler warning - not all
+			 * enum values call this function, so an incomplete list
+			 * is quite ok (but then we should not run into this code,
+			 * so at least we log a debug warning).
+			 */
+			dprintf("INTERNAL ERROR: doFileCreateModeUmaskLine() called with invalid eDir %d.\n",
+				eDir);
+			break;
+	}
+#endif
+
+	p += 4;	/* eat the octal number */
+	*pp = p;
+
+finalize_it:
+	return iRet;
+}
+
+
 /* Parse and interpret an on/off inside a config file line. This is most
  * often used for boolean options, but of course it may also be used
  * for other things. The passed-in pointer is updated to point to
@@ -90,7 +178,6 @@ rsRetVal doGetGID(uchar **pp, rsRetVal (*pSetHdlr)(void*, uid_t), void *pVal)
 
 	assert(pp != NULL);
 	assert(*pp != NULL);
-	assert(pVal != NULL);
 
 	if(getSubString(pp, (char*) szName, sizeof(szName) / sizeof(uchar), ' ')  != 0) {
 		logerror("could not extract group name");
@@ -109,7 +196,7 @@ rsRetVal doGetGID(uchar **pp, rsRetVal (*pSetHdlr)(void*, uid_t), void *pVal)
 			*((gid_t*)pVal) = pgBuf->gr_gid;
 		} else {
 			/* we set value via a set function */
-			pSetHdlr(pVal, pgBuf->gr_gid);
+			CHKiRet(pSetHdlr(pVal, pgBuf->gr_gid));
 		}
 		dprintf("gid %d obtained for group '%s'\n", pgBuf->gr_gid, szName);
 	}
@@ -134,7 +221,6 @@ rsRetVal doGetUID(uchar **pp, rsRetVal (*pSetHdlr)(void*, uid_t), void *pVal)
 
 	assert(pp != NULL);
 	assert(*pp != NULL);
-	assert(pVal != NULL);
 
 	if(getSubString(pp, (char*) szName, sizeof(szName) / sizeof(uchar), ' ')  != 0) {
 		logerror("could not extract user name");
@@ -153,7 +239,7 @@ rsRetVal doGetUID(uchar **pp, rsRetVal (*pSetHdlr)(void*, uid_t), void *pVal)
 			*((uid_t*)pVal) = ppwBuf->pw_uid;
 		} else {
 			/* we set value via a set function */
-			pSetHdlr(pVal, ppwBuf->pw_uid);
+			CHKiRet(pSetHdlr(pVal, ppwBuf->pw_uid));
 		}
 		dprintf("uid %d obtained for user '%s'\n", ppwBuf->pw_uid, szName);
 	}
@@ -177,7 +263,6 @@ rsRetVal doBinaryOptionLine(uchar **pp, rsRetVal (*pSetHdlr)(void*, int), void *
 
 	assert(pp != NULL);
 	assert(*pp != NULL);
-	assert(pVal != NULL);
 
 	if((iOption = doParseOnOffOption(pp)) == -1)
 		return RS_RET_ERR;	/* nothing left to do */
@@ -187,10 +272,12 @@ rsRetVal doBinaryOptionLine(uchar **pp, rsRetVal (*pSetHdlr)(void*, int), void *
 		*((int*)pVal) = iOption;
 	} else {
 		/* we set value via a set function */
-		pSetHdlr(pVal, iOption);
+		CHKiRet(pSetHdlr(pVal, iOption));
 	}
 
 	skipWhiteSpace(pp); /* skip over any whitespace */
+
+finalize_it:
 	return iRet;
 }
 
diff --git a/cfsysline.h b/cfsysline.h
index 66d01bed..bc1fd71f 100644
--- a/cfsysline.h
+++ b/cfsysline.h
@@ -69,5 +69,7 @@ rsRetVal cslchCallHdlr(cslCmdHdlr_t *pThis, uchar **ppConfLine);
 rsRetVal doBinaryOptionLine(uchar **pp, rsRetVal (*pSetHdlr)(void*, int), void *pVal);
 rsRetVal doGetUID(uchar **pp, rsRetVal (*pSetHdlr)(void*, uid_t), void *pVal);
 rsRetVal doGetGID(uchar **pp, rsRetVal (*pSetHdlr)(void*, uid_t), void *pVal);
+//void doFileCreateModeUmaskLine(uchar **pp, enum eDirective eDir);
+rsRetVal doFileCreateMode(uchar **pp, rsRetVal (*pSetHdlr)(void*, uid_t), void *pVal);
 
 #endif /* #ifndef CFSYSLINE_H_INCLUDED */
diff --git a/rsyslog.h b/rsyslog.h
index 149e589c..2badea3c 100644
--- a/rsyslog.h
+++ b/rsyslog.h
@@ -59,10 +59,16 @@ enum rsRetVal_				/** return value. All methods return this if not specified oth
 	RS_RET_DISABLE_ACTION = -2006,  /**< action requests that it be disabled */
 	RS_RET_SUSPENDED = -2007,  /**< something was suspended, not neccesarily an error */
 	RS_RET_RQD_TPLOPT_MISSING = -2008,/**< a required template option is missing */
+	RS_RET_INVALID_VALUE = -2009,/**< some value is invalid (e.g. user-supplied data) */
 	RS_RET_OK = 0			/**< operation successful */
 };
 typedef enum rsRetVal_ rsRetVal; /**< friendly type for global return value */
 
+/* some helpful macros to work with srRetVals.
+ * Be sure to call the to-be-returned variable always "iRet" and
+ * the function finalizer always "finalize_it".
+ */
+#define CHKiRet(code) if((iRet = code) != RS_RET_OK) goto finalize_it
 
 /** Object ID. These are for internal checking. Each
  * object is assigned a specific ID. This is contained in
diff --git a/syslogd.c b/syslogd.c
index 3743fa72..65528ea6 100644
--- a/syslogd.c
+++ b/syslogd.c
@@ -3533,78 +3533,6 @@ static void doControlCharEscPrefix(uchar **pp)
 	skipWhiteSpace(pp); /* skip over any whitespace */
 }
 
-
-/* Parse and interpet a $FileCreateMode and $umask line. This function
- * pulls the creation mode and, if successful, stores it
- * into the global variable so that the rest of rsyslogd
- * opens files with that mode. Any previous value will be
- * overwritten.
- * HINT: if we store the creation mode in selector_t, we
- * can even specify multiple modes simply be virtue of
- * being placed in the right section of rsyslog.conf
- * rgerhards, 2007-07-4 (happy independence day to my US friends!)
- * Parameter **pp has a pointer to the current config line.
- * On exit, it will be updated to the processed position.
- */
-static void doFileCreateModeUmaskLine(uchar **pp, enum eDirective eDir)
-{
-	uchar *p;
-	uchar errMsg[128];	/* for dynamic error messages */
-	int iMode;	
-
-	assert(pp != NULL);
-	assert(*pp != NULL);
-	
-	skipWhiteSpace(pp); /* skip over any whitespace */
-	p = *pp;
-
-	/* for now, we parse and accept only octal numbers
-	 * Sequence of tests is important, we are using boolean shortcuts
-	 * to avoid addressing invalid memory!
-	 */
-	if(!(   (*p == '0')
-	     && (*(p+1) && *(p+1) >= '0' && *(p+1) <= '7')
-	     && (*(p+2) && *(p+2) >= '0' && *(p+2) <= '7')
-	     && (*(p+3) && *(p+3) >= '0' && *(p+3) <= '7')  )  ) {
-		snprintf((char*) errMsg, sizeof(errMsg)/sizeof(uchar),
-		         "%s value must be octal (e.g 0644), invalid value '%s'.",
-			 eDir == DIR_UMASK ? "umask" : "filecreatemode", p);
-		errno = 0;
-		logerror((char*) errMsg);
-		return;
-	}
-
-	/*  we reach this code only if the octal number is ok - so we can now
-	 *  compute the value.
-	 */
-	iMode  = (*(p+1)-'0') * 64 + (*(p+2)-'0') * 8 + (*(p+3)-'0');
-	switch(eDir) {
-		case DIR_DIRCREATEMODE:
-			fDirCreateMode = iMode;
-			dprintf("DirCreateMode set to 0%o.\n", iMode);
-			break;
-		case DIR_FILECREATEMODE:
-			fCreateMode = iMode;
-			dprintf("FileCreateMode set to 0%o.\n", iMode);
-			break;
-		case DIR_UMASK:
-			umask(iMode);
-			dprintf("umask set to 0%3.3o.\n", iMode);
-			break;
-		default:/* we do this to avoid compiler warning - not all
-			 * enum values call this function, so an incomplete list
-			 * is quite ok (but then we should not run into this code,
-			 * so at least we log a debug warning).
-			 */
-			dprintf("INTERNAL ERROR: doFileCreateModeUmaskLine() called with invalid eDir %d.\n",
-				eDir);
-			break;
-	}
-
-	p += 4;	/* eat the octal number */
-	*pp = p;
-}
-
 /* parse and interpret a $-config line that starts with
  * a name (this is common code). It is parsed to the name
  * and then the proper sub-function is called to handle
@@ -3664,6 +3592,17 @@ static void doNameLine(uchar **pp, enum eDirective eDir)
 }
 
 
+/* set the processes umask (upon configuration request)
+ */
+static rsRetVal setUmask(void __attribute__((unused)) *pVal, int iUmask)
+{
+	umask(iUmask);
+	dprintf("umask set to 0%3.3o.\n", iUmask);
+
+	return RS_RET_OK;
+}
+
+
 /* Parse and interpret a system-directive in the config line
  * A system directive is one that starts with a "$" sign. It offers
  * extended configuration parameters.
@@ -3690,11 +3629,11 @@ void cfsysline(uchar *p)
 	} else if(!strcasecmp((char*) szCmd, "allowedsender")) { 
 		doNameLine(&p, DIR_ALLOWEDSENDER);
 	} else if(!strcasecmp((char*) szCmd, "dircreatemode")) { 
-		doFileCreateModeUmaskLine(&p, DIR_DIRCREATEMODE);
+		doFileCreateMode(&p, NULL, &fDirCreateMode);
 	} else if(!strcasecmp((char*) szCmd, "filecreatemode")) { 
-		doFileCreateModeUmaskLine(&p, DIR_FILECREATEMODE);
+		doFileCreateMode(&p, NULL, &fCreateMode);
 	} else if(!strcasecmp((char*) szCmd, "umask")) { 
-		doFileCreateModeUmaskLine(&p, DIR_UMASK);
+		doFileCreateMode(&p, (void*) setUmask, NULL);
 	} else if(!strcasecmp((char*) szCmd, "dirowner")) { 
 		doGetUID(&p, NULL, &dirUID);
 	} else if(!strcasecmp((char*) szCmd, "dirgroup")) {