From 06ffec1c3f9e566993d372cc686c8ae7307c5de0 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 20 Mar 2008 09:31:05 +0000 Subject: bugfix: fixed some minor memory leaks --- ChangeLog | 1 + cfsysline.c | 2 ++ debug.c | 4 ++++ modules.c | 8 +++++++- plugins/imfile/imfile.c | 4 +++- queue.c | 6 +++--- syslogd.c | 26 +++++++++++++++++++++++--- tcpsrv.c | 22 ++++++++++++++++------ 8 files changed, 59 insertions(+), 14 deletions(-) diff --git a/ChangeLog b/ChangeLog index bb1ac1ba..9f955eea 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,7 @@ Version 3.12.4 (rgerhards), 2008-03-?? varmojfekoj for the patch - bugfix: potential segfault on module unload. Thanks to varmojfekoj for the patch +- bugfix: fixed some minor memory leaks --------------------------------------------------------------------------- Version 3.12.3 (rgerhards), 2008-03-18 - added advanced flow control for congestion cases (mode depending on message diff --git a/cfsysline.c b/cfsysline.c index 013999a1..8f0439ed 100644 --- a/cfsysline.c +++ b/cfsysline.c @@ -487,6 +487,8 @@ finalize_it: * no custom handler is defined. If it is, the customer handler * must do the cleanup. I have checked and this was al also memory * leak with some code. Obviously, not a large one. -- rgerhards, 2007-12-20 + * Just to clarify: if pVal is parsed to a custom handler, this handler + * is responsible for freeing pVal. -- rgerhards, 2008-03-20 */ static rsRetVal doGetWord(uchar **pp, rsRetVal (*pSetHdlr)(void*, uchar*), void *pVal) { diff --git a/debug.c b/debug.c index 44f8bbd0..74dffe9f 100644 --- a/debug.c +++ b/debug.c @@ -657,6 +657,10 @@ static void dbgCallStackDestruct(void *arg) dbgThrdInfo_t *pThrd = (dbgThrdInfo_t*) arg; dbgprintf("destructor for debug call stack %p called\n", pThrd); + if(pThrd->pszThrdName != NULL) { + free(pThrd->pszThrdName); + } + DLL_Del(CallStack, pThrd); } diff --git a/modules.c b/modules.c index e6558a7e..98d8f281 100644 --- a/modules.c +++ b/modules.c @@ -188,8 +188,14 @@ static void moduleDestruct(modInfo_t *pThis) assert(pThis != NULL); if(pThis->pszName != NULL) free(pThis->pszName); - if(pThis->pModHdlr != NULL) + if(pThis->pModHdlr != NULL) { +# if 0 dlclose(pThis->pModHdlr); +# else +# warning "dlclose disabled for valgrind, re-enable before release" +# endif + } + free(pThis); } diff --git a/plugins/imfile/imfile.c b/plugins/imfile/imfile.c index ea1b03ad..a6e019d6 100644 --- a/plugins/imfile/imfile.c +++ b/plugins/imfile/imfile.c @@ -408,11 +408,13 @@ static rsRetVal resetConfigVariables(uchar __attribute__((unused)) *pp, void __a /* add a new monitor */ -static rsRetVal addMonitor(void __attribute__((unused)) *pVal, uchar __attribute__((unused)) *pNewVal) +static rsRetVal addMonitor(void __attribute__((unused)) *pVal, uchar *pNewVal) { DEFiRet; fileInfo_t *pThis; + free(pNewVal); /* we do not need it, but we must free it! */ + if(iFilPtr < MAX_INPUT_FILES) { pThis = &files[iFilPtr]; /* TODO: check for strdup() NULL return */ diff --git a/queue.c b/queue.c index acf21e32..ed720c55 100644 --- a/queue.c +++ b/queue.c @@ -827,9 +827,6 @@ static rsRetVal qDestructDisk(queue_t *pThis) strmDestruct(&pThis->tVars.disk.pWrite); strmDestruct(&pThis->tVars.disk.pRead); - if(pThis->pszSpoolDir != NULL) - free(pThis->pszSpoolDir); - RETiRet; } @@ -1926,6 +1923,9 @@ CODESTARTobjDestruct(queue) if(pThis->pszFilePrefix != NULL) free(pThis->pszFilePrefix); + + if(pThis->pszSpoolDir != NULL) + free(pThis->pszSpoolDir); ENDobjDestruct(queue) diff --git a/syslogd.c b/syslogd.c index 454083d4..9e7f80cb 100644 --- a/syslogd.c +++ b/syslogd.c @@ -1872,6 +1872,23 @@ static void doDie(int sig) } +/* This function frees all dynamically allocated memory for program termination. + * It must be called only immediately before exit(). It is primarily an aid + * for memory debuggers, which prevents cluttered outupt. + * rgerhards, 2008-03-20 + */ +static void +freeAllDynMemForTermination(void) +{ + if(pszWorkDir != NULL) + free(pszWorkDir); + if(pszMainMsgQFName != NULL) + free(pszMainMsgQFName); + if(pModDir != NULL) + free(pModDir); +} + + /* die() is called when the program shall end. This typically only occurs * during sigterm or during the initialization. * As die() is intended to shutdown rsyslogd, it is @@ -1947,9 +1964,6 @@ die(int sig) */ unregCfSysLineHdlrs(); - /* clean up auxiliary data */ - if(pModDir != NULL) - free(pModDir); legacyOptsFree(); /* terminate the remaining classes */ @@ -1971,6 +1985,12 @@ die(int sig) /* dbgClassExit MUST be the last one, because it de-inits the debug system */ dbgClassExit(); + /* free all remaining memory blocks - this is not absolutely necessary, but helps + * us keep memory debugger logs clean and this is in aid in developing. It doesn't + * cost much time, so we do it always. -- rgerhards, 2008-03-20 + */ + freeAllDynMemForTermination(); + /* NO CODE HERE - feeelAllDynMemForTermination() must be the last thing before exit()! */ exit(0); /* "good" exit, this is the terminator function for rsyslog [die()] */ } diff --git a/tcpsrv.c b/tcpsrv.c index 4553656e..165cbc69 100644 --- a/tcpsrv.c +++ b/tcpsrv.c @@ -110,8 +110,8 @@ static void freeAllSockets(int **socks) * NOTE: you can not use dbgprintf() in here - the dbgprintf() system is * not yet initilized when this function is called. * rgerhards, 2007-06-21 - * We can also not use errmsg.LogError(NO_ERRCODE, ), as that system is also not yet - * initialized... rgerhards, 2007-06-28 + * The port in cOptarg is handed over to us - the caller MUST NOT free it! + * rgerhards, 2008-03-20 */ static void configureTCPListen(tcpsrv_t *pThis, char *cOptarg) @@ -128,11 +128,15 @@ configureTCPListen(tcpsrv_t *pThis, char *cOptarg) i = i * 10 + *pArg++ - '0'; } + if(pThis->TCPLstnPort != NULL) { + free(pThis->TCPLstnPort); + pThis->TCPLstnPort = NULL; + } + if( i >= 0 && i <= 65535) { pThis->TCPLstnPort = cOptarg; } else { errmsg.LogError(NO_ERRCODE, "Invalid TCP listen port %s - changed to 514.\n", cOptarg); - pThis->TCPLstnPort = "514"; } } @@ -253,6 +257,9 @@ static void deinit_tcp_listener(tcpsrv_t *pThis) free(pThis->pSessions); pThis->pSessions = NULL; /* just to make sure... */ + if(pThis->TCPLstnPort != NULL) + free(pThis->TCPLstnPort); + /* finally close the listen sockets themselfs */ freeAllSockets(&pThis->pSocksLstn); } @@ -274,24 +281,27 @@ static int *create_tcp_socket(tcpsrv_t *pThis) { struct addrinfo hints, *res, *r; int error, maxs, *s, *socks, on = 1; + char *TCPLstnPort; ISOBJ_TYPE_assert(pThis, tcpsrv); if(!strcmp(pThis->TCPLstnPort, "0")) - pThis->TCPLstnPort = "514"; + TCPLstnPort = "514"; /* use default - we can not do service db update, because there is * no IANA-assignment for syslog/tcp. In the long term, we might * re-use RFC 3195 port of 601, but that would probably break to * many existing configurations. * rgerhards, 2007-06-28 */ - dbgprintf("creating tcp socket on port %s\n", pThis->TCPLstnPort); + else + TCPLstnPort = pThis->TCPLstnPort; + dbgprintf("creating tcp socket on port %s\n", TCPLstnPort); memset(&hints, 0, sizeof(hints)); hints.ai_flags = AI_PASSIVE | AI_NUMERICSERV; hints.ai_family = family; hints.ai_socktype = SOCK_STREAM; - error = getaddrinfo(NULL, pThis->TCPLstnPort, &hints, &res); + error = getaddrinfo(NULL, TCPLstnPort, &hints, &res); if(error) { errmsg.LogError(NO_ERRCODE, "%s", gai_strerror(error)); return NULL; -- cgit