Note: there are also official repositories on github for most of the repositories present here. Please consider contributing to rsyslog.
For more information read Where to find the rsyslog source code, which applies to rsyslog-related projects (like librelp) as well.

bugfix: fixed a memory leak and potential abort condition
authorRainer Gerhards <rgerhards@adiscon.com>
Fri, 25 Feb 2011 13:14:17 +0000 (14:14 +0100)
committerRainer Gerhards <rgerhards@adiscon.com>
Fri, 25 Feb 2011 13:14:17 +0000 (14:14 +0100)
this could happen if multiple rulesets were used and some output batches
contained messages belonging to more than one ruleset.
fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=226
fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=218

ChangeLog
runtime/batch.h
runtime/ruleset.c

index 5596dfb..c4cb28b 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 ---------------------------------------------------------------------------
 Version 5.7.6  [V5-BETA] (rgerhards), 2011-02-??
+- bugfix: fixed a memory leak and potential abort condition
+  this could happen if multiple rulesets were used and some output batches
+  contained messages belonging to more than one ruleset.
+  fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=226
+  fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=218
 - bugfix: memory leak when $RepeatedMsgReduction on was used
   bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=225
 ---------------------------------------------------------------------------
index d0504f2..944889b 100644 (file)
@@ -136,11 +136,16 @@ batchIsValidElem(batch_t *pBatch, int i) {
 /* copy one batch element to another.
  * This creates a complete duplicate in those cases where
  * it is needed. Use duplication only when absolutely necessary!
+ * Note that all working fields are reset to zeros. If that were 
+ * not done, we would have potential problems with invalid
+ * or double pointer frees.
  * rgerhards, 2010-06-10
  */
 static inline void
 batchCopyElem(batch_obj_t *pDest, batch_obj_t *pSrc) {
-       memcpy(pDest, pSrc, sizeof(batch_obj_t));
+       memset(pDest, 0, sizeof(batch_obj_t));
+       pDest->pUsrp = pSrc->pUsrp;
+       pDest->state = pSrc->state;
 }
 
 
@@ -171,6 +176,7 @@ batchFree(batch_t *pBatch) {
 static inline rsRetVal
 batchInit(batch_t *pBatch, int maxElem) {
        DEFiRet;
+       pBatch->iDoneUpTo = 0;
        pBatch->maxElem = maxElem;
        CHKmalloc(pBatch->pElem = calloc((size_t)maxElem, sizeof(batch_obj_t)));
        // TODO: replace calloc by inidividual writes?
index 0584e8d..c9c64a3 100644 (file)
@@ -171,35 +171,40 @@ processBatchMultiRuleset(batch_t *pBatch)
        int i;
        int iStart;     /* start index of partial batch */
        int iNew;       /* index for new (temporary) batch */
+       int bHaveUnprocessed;   /* do we (still) have unprocessed entries? (loop term predicate) */
        DEFiRet;
 
-       CHKiRet(batchInit(&snglRuleBatch, pBatch->nElem));
-       snglRuleBatch.pbShutdownImmediate = pBatch->pbShutdownImmediate;
-
-       while(1) { /* loop broken inside */
+       do {
+               bHaveUnprocessed = 0;
                /* search for first unprocessed element */
                for(iStart = 0 ; iStart < pBatch->nElem && pBatch->pElem[iStart].state == BATCH_STATE_DISC ; ++iStart)
                        /* just search, no action */;
-
                if(iStart == pBatch->nElem)
-                       FINALIZE; /* everything processed */
+                       break; /* everything processed */
 
                /* prepare temporary batch */
+               CHKiRet(batchInit(&snglRuleBatch, pBatch->nElem));
+               snglRuleBatch.pbShutdownImmediate = pBatch->pbShutdownImmediate;
                currRuleset = batchElemGetRuleset(pBatch, iStart);
                iNew = 0;
                for(i = iStart ; i < pBatch->nElem ; ++i) {
                        if(batchElemGetRuleset(pBatch, i) == currRuleset) {
-                               batchCopyElem(&(snglRuleBatch.pElem[iNew++]), &(pBatch->pElem[i]));
+                               /* for performance reasons, we copy only those members that we actually need */
+                               snglRuleBatch.pElem[iNew].pUsrp = pBatch->pElem[i].pUsrp;
+                               snglRuleBatch.pElem[iNew].state = pBatch->pElem[i].state;
+                               ++iNew;
                                /* We indicate the element also as done, so it will not be processed again */
                                pBatch->pElem[i].state = BATCH_STATE_DISC;
+                       } else {
+                               bHaveUnprocessed = 1;
                        }
                }
                snglRuleBatch.nElem = iNew; /* was left just right by the for loop */
                batchSetSingleRuleset(&snglRuleBatch, 1);
                /* process temp batch */
                processBatch(&snglRuleBatch);
-       }
-       batchFree(&snglRuleBatch);
+               batchFree(&snglRuleBatch);
+       } while(bHaveUnprocessed == 1);
 
 finalize_it:
        RETiRet;