From 214c7bd7f8552f0a07a15373b4a222da6e9e6ba6 Mon Sep 17 00:00:00 2001
From: Rainer Gerhards <rgerhards@adiscon.com>
Date: Wed, 21 Nov 2007 10:09:02 +0000
Subject: added new modExit() entry point to loadable module interface

---
 ChangeLog                   |  8 +++++--
 action.h                    |  2 +-
 configure.ac                |  2 +-
 module-template.h           | 51 ++++++++++++++++++++++++++++++++++++++++++---
 modules.c                   | 26 ++++++++++++++++++-----
 omdiscard.c                 |  5 +++++
 omfile.c                    |  6 +++++-
 omfwd.c                     |  5 +++++
 omshell.c                   |  5 +++++
 omusrmsg.c                  |  5 +++++
 plugins/ommysql/Makefile.am |  2 +-
 plugins/ommysql/ommysql.c   |  5 +++++
 syslogd.c                   |  9 ++++----
 13 files changed, 113 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b8c67fc0..0654da03 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,10 @@
 ---------------------------------------------------------------------------
-Version 1.19.11 (rgerhards), 2007-10-
-- ...
+Version 1.20.0 (rgerhards), 2007-10-
+- GSS-API support for syslog/TCP connections was added. Thanks to
+  varmojfekoj for providing the patch with this functionality
+- unloading dynamic modules has been cleaned up, we now have a
+  real implementation and not just a dummy "good enough for the time
+  being".
 ---------------------------------------------------------------------------
 Version 1.19.10 (rgerhards), 2007-10-19
 - introdcued the new ":modulename:" syntax for calling module actions
diff --git a/action.h b/action.h
index e7757d73..c0cf19ad 100644
--- a/action.h
+++ b/action.h
@@ -37,7 +37,7 @@ struct action_s {
 	int	iResumeInterval;/* resume interval for this action */
 	int	iNbrResRtry;	/* number of retries since last suspend */
 	struct moduleInfo *pMod;/* pointer to output module handling this selector */
-	void	*pModData;	/* pointer to module data - contents is module-specific */
+	void	*pModData;	/* pointer to module data - content is module-specific */
 	int	f_ReduceRepeated;/* reduce repeated lines 0 - no, 1 - yes */
 	int	f_prevcount;	/* repetition cnt of prevline */
 	int	f_repeatcount;	/* number of "repeated" msgs */
diff --git a/configure.ac b/configure.ac
index f146259d..20fee5c7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,7 +2,7 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ(2.61)
-AC_INIT([rsyslog],[1.19.11],[rsyslog@lists.adiscon.com.])
+AC_INIT([rsyslog],[1.20.0],[rsyslog@lists.adiscon.com.])
 AM_INIT_AUTOMAKE
 AC_CONFIG_SRCDIR([syslogd.c])
 AC_CONFIG_HEADER([config.h])
diff --git a/module-template.h b/module-template.h
index e3fe06de..efe977d7 100644
--- a/module-template.h
+++ b/module-template.h
@@ -63,6 +63,13 @@ static rsRetVal createInstance(instanceData **ppData)\
 }
 
 /* freeInstance()
+ * This is the cleanup function for the module instance. It is called immediately before
+ * the module instance is destroyed (unloaded). The module should do any cleanup
+ * here, e.g. close file, free instantance heap memory and the like. Control will
+ * not be passed back to the module once this function is finished. Keep in mind,
+ * however, that other instances may still be loaded and used. So do not destroy
+ * anything that may be used by another instance. If you have such a ressource, you
+ * currently need to do the instance counting yourself.
  */
 #define BEGINfreeInstance \
 static rsRetVal freeInstance(void* pModData)\
@@ -284,7 +291,7 @@ static rsRetVal queryEtryPt(uchar *name, rsRetVal (**pEtryPoint)())\
  * modules. This can be used if no specific handling (e.g. to cover version
  * differences) is needed.
  */
-#define CODEqueryEtryPt_STD_OMOD_QUERIES\
+#define CODEqueryEtryPt_STD_OMOD_QUERIES \
 	if(!strcmp((char*) name, "doAction")) {\
 		*pEtryPoint = doAction;\
 	} else if(!strcmp((char*) name, "parseSelectorAct")) {\
@@ -295,6 +302,8 @@ static rsRetVal queryEtryPt(uchar *name, rsRetVal (**pEtryPoint)())\
 		*pEtryPoint = dbgPrintInstInfo;\
 	} else if(!strcmp((char*) name, "freeInstance")) {\
 		*pEtryPoint = freeInstance;\
+	} else if(!strcmp((char*) name, "modExit")) {\
+		*pEtryPoint = modExit;\
 	} else if(!strcmp((char*) name, "getWriteFDForSelect")) {\
 		*pEtryPoint = getWriteFDForSelect;\
 	} else if(!strcmp((char*) name, "onSelectReadyWrite")) {\
@@ -305,10 +314,18 @@ static rsRetVal queryEtryPt(uchar *name, rsRetVal (**pEtryPoint)())\
 		*pEtryPoint = tryResume;\
 	}
 
+
 /* modInit()
  * This has an extra parameter, which is the specific name of the modInit
  * function. That is needed for built-in modules, which must have unique
- * names in order to link statically.
+ * names in order to link statically. Please note that this is alwaysy only
+ * the case with modInit() and NO other entry point. The reason is that only
+ * modInit() is visible form a linker/loader point of view. All other entry
+ * points are passed via rsyslog-internal query functions and are defined
+ * static inside the modules source. This is an important concept, as it allows
+ * us to support different interface versions within a single module. (Granted,
+ * we do not currently have different interface versions, so we can not put
+ * it to a test - but our firm believe is that we can do all abstraction needed...)
  *
  * Extra Comments:
  * initialize the module
@@ -316,9 +333,12 @@ static rsRetVal queryEtryPt(uchar *name, rsRetVal (**pEtryPoint)())\
  * Later, much more must be done. So far, we only return a pointer
  * to the queryEtryPt() function
  * TODO: do interface version checking & handshaking
- * iIfVersRequeted is the version of the interface specification that the
+ * iIfVersRequetsed is the version of the interface specification that the
  * caller would like to see being used. ipIFVersProvided is what we
  * decide to provide.
+ * rgerhards, 2007-11-21: see modExit() comment below for important information
+ * on the need to initialize static data with code. modInit() may be called on a
+ * cached, left-in-memory copy of a previous incarnation.
  */
 #define BEGINmodInit(uniqName) \
 rsRetVal modInit##uniqName(int iIFVersRequested __attribute__((unused)), int *ipIFVersProvided, rsRetVal (**pQueryEtryPt)(), rsRetVal (*pHostQueryEtryPt)(uchar*, rsRetVal (**)()))\
@@ -342,6 +362,31 @@ finalize_it:\
 	CHKiRet(pHostQueryEtryPt((uchar*)"regCfSysLineHdlr", &omsdRegCFSLineHdlr));
 
 #endif /* #ifndef MODULE_TEMPLATE_H_INCLUDED */
+
+/* modExit()
+ * This is the counterpart to modInit(). It destroys a module and makes it ready for
+ * unloading. It is similiar to freeInstance() for the instance data. Please note that
+ * this entry point needs to free any module-globale data structures and registrations.
+ * For example, the CfSysLineHandlers a module has registered need to be unregistered
+ * here. This entry point is only called immediately before unloading of the module. So
+ * it is likely to be destroyed. HOWEVER, the caller may decide to keep the module cached.
+ * So a module must never assume that it is actually destroyed. A call to modInit() may
+ * happen immediately after modExit(). So a module can NOT assume that static data elements
+ * are being re-initialized by the loader - this must always be done by module code itself.
+ * It is suggested to do this in modInit(). - rgerhards, 2007-11-21
+ */
+#define BEGINmodExit \
+static rsRetVal modExit(void)\
+{\
+dbgprintf("in modExit\n");\
+	DEFiRet;
+
+#define CODESTARTmodExit 
+
+#define ENDmodExit \
+	return iRet;\
+}
+
 /*
  * vi:set ai:
  */
diff --git a/modules.c b/modules.c
index 11aad034..b92f08c3 100644
--- a/modules.c
+++ b/modules.c
@@ -42,7 +42,7 @@
 #include "modules.h"
 
 static modInfo_t *pLoadedModules = NULL;	/* list of currently-loaded modules */
-static modInfo_t *pLoadedModulesLast = NULL; /* tail-pointer */
+static modInfo_t *pLoadedModulesLast = NULL;	/* tail-pointer */
 static int bCfsyslineInitialized = 0;
 
 
@@ -64,8 +64,9 @@ static rsRetVal moduleConstruct(modInfo_t **pThis)
 }
 
 
-/* Destructs a module objects. The object must not be linked to the
- * linked list of modules.
+/* Destructs a module object. The object must not be linked to the
+ * linked list of modules. Please note that all other dependencies on this
+ * modules must have been removed before (e.g. CfSysLineHandlers!)
  */
 static void moduleDestruct(modInfo_t *pThis)
 {
@@ -172,8 +173,13 @@ modInfo_t *omodGetNxt(modInfo_t *pThis)
  * (builtin) module, nothing happens.
  * The module handle is invalid after this function call and 
  * MUST NOT be used any more.
- * This is currently a dummy, to be filled when we have a plug-in interface
- * rgerhards, 2007-08-09
+ * This is currently a dummy, to be filled when we have a plug-in
+ * interface - rgerhards, 2007-08-09
+ * rgerhards, 2007-11-21:
+ * When this function is called, all instance-data must already have
+ * been destroyed. In the case of output modules, this happens when the
+ * rule set is being destroyed. When we implement other module types, we
+ * need to think how we handle it there (and if we have any instance data).
  */
 static rsRetVal modUnload(modInfo_t *pThis)
 {
@@ -186,6 +192,11 @@ static rsRetVal modUnload(modInfo_t *pThis)
 	}
 
 	/* TODO: implement code */
+	/* There is a bunch of things we need to do:
+	 * - unregister this modules config handler
+	 * - unload the module itself
+	 * - think about the instances freeInstance()
+	 */
 	ABORT_FINALIZE(RS_RET_NOT_IMPLEMENTED);
 
 finalize_it:
@@ -263,6 +274,10 @@ rsRetVal doModInit(rsRetVal (*modInit)(int, int*, rsRetVal(**)(), rsRetVal(*)())
 		moduleDestruct(pNew);
 		return iRet;
 	}
+	if((iRet = (*pNew->modQueryEtryPt)((uchar*)"modExit", &pNew->modExit)) != RS_RET_OK) {
+		moduleDestruct(pNew);
+		return iRet;
+	}
 
 	pNew->pszName = (uchar*) strdup((char*)name); /* we do not care if strdup() fails, we can accept that */
 	pNew->pModHdlr = pModHdlr;
@@ -355,6 +370,7 @@ rsRetVal modUnloadAndDestructDynamic(void)
 		/* now we can destroy the previous module */
 		if(pModPrev->eLinkType != eMOD_LINK_STATIC) {
 			dbgprintf("Unloading module %s\n", modGetName(pModPrev));
+			modUnload(pModPrev);
 			moduleDestruct(pModPrev);
 		} else {
 			pLoadedModulesLast = pModPrev;
diff --git a/omdiscard.c b/omdiscard.c
index d3350032..bcd9ba21 100644
--- a/omdiscard.c
+++ b/omdiscard.c
@@ -111,6 +111,11 @@ CODESTARTgetWriteFDForSelect
 ENDgetWriteFDForSelect
 
 
+BEGINmodExit
+CODESTARTmodExit
+ENDmodExit
+
+
 BEGINqueryEtryPt
 CODESTARTqueryEtryPt
 CODEqueryEtryPt_STD_OMOD_QUERIES
diff --git a/omfile.c b/omfile.c
index d6cfa86a..5bc2a811 100644
--- a/omfile.c
+++ b/omfile.c
@@ -792,6 +792,11 @@ static rsRetVal resetConfigVariables(uchar __attribute__((unused)) *pp, void __a
 }
 
 
+BEGINmodExit
+CODESTARTmodExit
+ENDmodExit
+
+
 BEGINqueryEtryPt
 CODESTARTqueryEtryPt
 CODEqueryEtryPt_STD_OMOD_QUERIES
@@ -813,7 +818,6 @@ CODEmodInit_QueryRegCFSLineHdlr
 	CHKiRet(omsdRegCFSLineHdlr((uchar *)"failonchownfailure", 0, eCmdHdlrBinary, NULL, &bFailOnChown));
 	CHKiRet(omsdRegCFSLineHdlr((uchar *)"resetconfigvariables", 1, eCmdHdlrCustomHandler, resetConfigVariables, NULL));
 ENDmodInit
-
 /*
  * vi:set ai:
  */
diff --git a/omfwd.c b/omfwd.c
index 49c59fa3..7719e05f 100644
--- a/omfwd.c
+++ b/omfwd.c
@@ -1129,6 +1129,11 @@ CODESTARTgetWriteFDForSelect
 ENDgetWriteFDForSelect
 
 
+BEGINmodExit
+CODESTARTmodExit
+ENDmodExit
+
+
 BEGINqueryEtryPt
 CODESTARTqueryEtryPt
 CODEqueryEtryPt_STD_OMOD_QUERIES
diff --git a/omshell.c b/omshell.c
index 54120cae..41e28991 100644
--- a/omshell.c
+++ b/omshell.c
@@ -134,6 +134,11 @@ CODESTARTgetWriteFDForSelect
 ENDgetWriteFDForSelect
 
 
+BEGINmodExit
+CODESTARTmodExit
+ENDmodExit
+
+
 BEGINqueryEtryPt
 CODESTARTqueryEtryPt
 CODEqueryEtryPt_STD_OMOD_QUERIES
diff --git a/omusrmsg.c b/omusrmsg.c
index aa8fc8ce..03d8b7f0 100644
--- a/omusrmsg.c
+++ b/omusrmsg.c
@@ -329,6 +329,11 @@ CODESTARTgetWriteFDForSelect
 ENDgetWriteFDForSelect
 
 
+BEGINmodExit
+CODESTARTmodExit
+ENDmodExit
+
+
 BEGINqueryEtryPt
 CODESTARTqueryEtryPt
 CODEqueryEtryPt_STD_OMOD_QUERIES
diff --git a/plugins/ommysql/Makefile.am b/plugins/ommysql/Makefile.am
index f388d1a9..ee4b12bf 100644
--- a/plugins/ommysql/Makefile.am
+++ b/plugins/ommysql/Makefile.am
@@ -1,7 +1,7 @@
 pkglibdir = $(libdir)/rsyslog
 pkglib_LTLIBRARIES = ommysql.la
 
-ommysql_la_SOURCES = ommysql.c ommysql.h
+ommysql_la_SOURCES = ommysql.c ommysql.h ../../module-template.h
 ommysql_la_CPPFLAGS = $(mysql_cflags) -I$(srcdir)/../..
 ommysql_la_LDFLAGS = -module -avoid-version
 ommysql_la_LIBADD = $(mysql_libs)
diff --git a/plugins/ommysql/ommysql.c b/plugins/ommysql/ommysql.c
index c132956a..33651a79 100644
--- a/plugins/ommysql/ommysql.c
+++ b/plugins/ommysql/ommysql.c
@@ -288,6 +288,11 @@ CODE_STD_FINALIZERparseSelectorAct
 ENDparseSelectorAct
 
 
+BEGINmodExit
+CODESTARTmodExit
+ENDmodExit
+
+
 BEGINqueryEtryPt
 CODESTARTqueryEtryPt
 CODEqueryEtryPt_STD_OMOD_QUERIES
diff --git a/syslogd.c b/syslogd.c
index 144f11d8..c0fd4034 100644
--- a/syslogd.c
+++ b/syslogd.c
@@ -3644,7 +3644,7 @@ static void die(int sig)
 	/* de-init some modules */
 	modExitIminternal();
 
-	unregCfSysLineHdlrs();
+	unregCfSysLineHdlrs(); /* TODO: this needs to go away when the module de-init works */
 
 	/* TODO: this would also be the right place to de-init the builtin output modules. We
 	 * do not currently do that, because the module interface does not allow for
@@ -4329,8 +4329,7 @@ static void init(void)
 		}
 	}
 
-	assert (pAllowedSenders_UDP == NULL &&
-		pAllowedSenders_TCP == NULL );
+	assert(pAllowedSenders_UDP == NULL && pAllowedSenders_TCP == NULL);
 #endif
 	/* I was told by an IPv6 expert that calling getservbyname() seems to be
 	 * still valid, at least for the use case we have. So I re-enabled that
@@ -4366,7 +4365,9 @@ static void init(void)
 	dbgprintf("rsyslog %s.\n", VERSION);
 	dbgprintf("Called init.\n");
 
-	/*  Close all open log files and free log descriptor array. */
+	/*  Close all open log files and free log descriptor array. This also frees
+	 *  all output-modules instance data.
+	 */
 	freeSelectors();
 
 	/* Unload all non-static modules */
-- 
cgit