libesi: Various fixes

* Mostly uninitialized class member variables

* One missing error check in ESI parser (740408)

* One potential NULL from dynamic cast fixed (740372)

* Several useless/dead code checks

 Detected by Coverity Scan. Issues 740545, 740408, 740546, 740372, 740547,
  740548, 740549, 740550, 740551, 740345, 740346, 740552, 740553, 740554,
  740555, 740556, 740557.

diff --git a/src/esi/Context.h b/src/esi/Context.h
index 7995c4a7d90e9c910e4577e630a188a455e8d7ed..baae3fdf979da27bae24a70376a7051a75c9829d 100644 (file)
--- a/src/esi/Context.h
+++ b/src/esi/Context.h
@@ -50,7 +50,22 @@ public:
     typedef RefCount<ESIContext> Pointer;
     void *operator new (size_t byteCount);
     void operator delete (void *address);
-    ESIContext():reading_(true) {}
+    ESIContext() :
+            thisNode(NULL),
+            http(NULL),
+            errorpage(ERR_NONE),
+            errorstatus(HTTP_STATUS_NONE),
+            errormessage(NULL),
+            rep(NULL),
+            outbound_offset(0),
+            readpos(0),
+            pos(0),
+            varState(NULL),
+            cachedASTInUse(false),
+            reading_(true),
+            processing(false) {
+        memset(&flags, 0, sizeof(flags));
+    }
 
     ~ESIContext();
 

diff --git a/src/esi/CustomParser.cc b/src/esi/CustomParser.cc
index bd5d414e0b097581b9220d02e637df3df825400c..4f1c0835f53f183eba242d7e60d719ac2c60954e 100644 (file)
--- a/src/esi/CustomParser.cc
+++ b/src/esi/CustomParser.cc
@@ -68,7 +68,9 @@ ESICustomParser::GetTrie()
     return SearchTrie;
 }
 
-ESICustomParser::ESICustomParser(ESIParserClient *aClient) : theClient (aClient)
+ESICustomParser::ESICustomParser(ESIParserClient *aClient) :
+        theClient(aClient),
+        lastTag(ESITAG)
 {}
 
 ESICustomParser::~ESICustomParser()
@@ -190,7 +192,14 @@ ESICustomParser::parse(char const *dataToParse, size_t const lengthOfData, bool
                 }
 
                 char *value = equals + 1;
-                char *end = strchr (value, sep);
+                char *end = strchr(value, sep);
+
+                if (!end) {
+                    error = "Missing attribute ending separator (";
+                    error.append(sep);
+                    error.append(")");
+                    return false;
+                }
                 attributes.push_back(value);
                 *end = '\0';
                 attribute = end + 1;

diff --git a/src/esi/Esi.cc b/src/esi/Esi.cc
index a863a4f1ea37a00aa9af0af62bdd193ea87f17a1..3f3bf5aa3f5e2ddb0819962914ba6f2aed0dea4d 100644 (file)
--- a/src/esi/Esi.cc
+++ b/src/esi/Esi.cc
@@ -968,7 +968,10 @@ ESIContext::ParserState::top()
     return stack[stackdepth-1];
 }
 
-ESIContext::ParserState::ParserState() : inited_ (false)
+ESIContext::ParserState::ParserState() :
+stackdepth(0),
+parsing(0),
+inited_(false)
 {}
 
 bool
@@ -1554,6 +1557,7 @@ esiLiteral::finish()
 esiLiteral::esiLiteral(ESIContext *context, const char *s, int numberOfCharacters)
 {
     assert (s);
+    flags.donevars = 0;
     buffer = new ESISegment;
     ESISegment::Pointer local = buffer;
     size_t start = 0;
@@ -1570,7 +1574,7 @@ esiLiteral::esiLiteral(ESIContext *context, const char *s, int numberOfCharacter
         remainingCharacters -= len;
     }
 
-    varState = cbdataReference (context->varState);
+    varState = cbdataReference(context->varState);
 }
 
 void
@@ -1705,11 +1709,15 @@ esiTry::~esiTry()
     debugs(86, 5, "esiTry::~esiTry " << this);
 }
 
-esiTry::esiTry(esiTreeParentPtr aParent) : parent (aParent) , exceptbuffer(NULL)
-{}
+esiTry::esiTry(esiTreeParentPtr aParent) :
+        parent(aParent),
+        exceptbuffer(NULL)
+{
+    memset(&flags, 0, sizeof(flags));
+}
 
 void
-esiTry::render (ESISegment::Pointer output)
+esiTry::render(ESISegment::Pointer output)
 {
     /* Try renders from it's children */
     assert (this);
@@ -2087,8 +2095,8 @@ esiChoose::selectElement()
         return;
 
     for (size_t counter = 0; counter < elements.size(); ++counter) {
-        if ((dynamic_cast<esiWhen *>(elements[counter].getRaw()))->
-                testsTrue()) {
+        const esiWhen *el = dynamic_cast<esiWhen *>(elements[counter].getRaw());
+        if (el && el->testsTrue()) {
             chosenelement = counter;
             debugs (86,3, "esiChooseAdd: Chose element " << counter + 1);
             return;
@@ -2325,9 +2333,12 @@ ElementList::size() const
 }
 
 /* esiWhen */
-esiWhen::esiWhen (esiTreeParentPtr aParent, int attrcount, const char **attr,ESIVarState *aVar) : esiSequence (aParent)
+esiWhen::esiWhen(esiTreeParentPtr aParent, int attrcount, const char **attr,ESIVarState *aVar) :
+        esiSequence(aParent),
+        testValue(false),
+        unevaluatedExpression(NULL),
+        varState(NULL)
 {
-    varState = NULL;
     char const *expression = NULL;
 
     for (int loopCounter = 0; loopCounter < attrcount && attr[loopCounter]; loopCounter += 2) {
@@ -2370,7 +2381,7 @@ esiWhen::evaluate()
     if (!unevaluatedExpression)
         return;
 
-    assert (varState);
+    assert(varState);
 
     varState->feedData(unevaluatedExpression, strlen (unevaluatedExpression));
 
@@ -2381,14 +2392,14 @@ esiWhen::evaluate()
     safe_free (expression);
 }
 
-esiWhen::esiWhen(esiWhen const &old) : esiSequence (old)
+esiWhen::esiWhen(esiWhen const &old) :
+        esiSequence(old),
+        testValue(NULL),
+        unevaluatedExpression(NULL),
+        varState(NULL)
 {
-    unevaluatedExpression = NULL;
-
     if (old.unevaluatedExpression)
         unevaluatedExpression = xstrdup(old.unevaluatedExpression);
-
-    varState = NULL;
 }
 
 ESIElement::Pointer

diff --git a/src/esi/Expression.cc b/src/esi/Expression.cc
index 48562fbdb347c08a3d3ffc7c1431a64a177515d8..b0a570fe36ad99d88ae8564f7926cee103ab4091 100644 (file)
--- a/src/esi/Expression.cc
+++ b/src/esi/Expression.cc
@@ -293,10 +293,6 @@ evalor(stackmember * stack, int *depth, int whereAmI, stackmember * candidate)
 
     rv = stack[whereAmI - 1].value.integral || stack[whereAmI + 1].value.integral;
 
-    if (rv == -2)
-        /* invalid comparison */
-        return 1;
-
     stackpop(stack, depth);      /* arg rhs */
 
     stackpop(stack, depth);      /* me */
@@ -344,10 +340,6 @@ evaland(stackmember * stack, int *depth, int whereAmI, stackmember * candidate)
 
     rv = stack[whereAmI - 1].value.integral && stack[whereAmI + 1].value.integral;
 
-    if (rv == -2)
-        /* invalid comparison */
-        return 1;
-
     stackpop(stack, depth);      /* arg rhs */
 
     stackpop(stack, depth);      /* me */

diff --git a/src/esi/Include.cc b/src/esi/Include.cc
index 1feaad09573f0aebda71e15b5d984bd09a7136f4..9da0211e14d879aa318d5ae4300e684e30850edc 100644 (file)
--- a/src/esi/Include.cc
+++ b/src/esi/Include.cc
@@ -299,9 +299,15 @@ ESIInclude::makeUsable(esiTreeParentPtr newParent, ESIVarState &newVarState) con
     return result;
 }
 
-ESIInclude::ESIInclude(ESIInclude const &old) : parent (NULL), started (false), sent (false)
+ESIInclude::ESIInclude(ESIInclude const &old) :
+        parent(NULL),
+        varState(NULL),
+        srcurl(NULL),
+        alturl(NULL),
+        started(false),
+        sent(false)
 {
-    varState = NULL;
+    memset(&flags, 0, sizeof(flags));
     flags.onerrorcontinue = old.flags.onerrorcontinue;
 
     if (old.srcurl)
@@ -343,12 +349,18 @@ ESIInclude::Start (ESIStreamContext::Pointer stream, char const *url, ESIVarStat
     tempheaders.clean();
 }
 
-ESIInclude::ESIInclude (esiTreeParentPtr aParent, int attrcount, char const **attr, ESIContext *aContext) : parent (aParent), started (false), sent (false)
+ESIInclude::ESIInclude(esiTreeParentPtr aParent, int attrcount, char const **attr, ESIContext *aContext) :
+        parent(aParent),
+        varState(NULL),
+        srcurl(NULL),
+        alturl(NULL),
+        started(false),
+        sent(false)
 {
-    int i;
     assert (aContext);
+    memset(&flags, 0, sizeof(flags));
 
-    for (i = 0; i < attrcount && attr[i]; i += 2) {
+    for (int i = 0; i < attrcount && attr[i]; i += 2) {
         if (!strcmp(attr[i],"src")) {
             /* Start a request for thisNode url */
             debugs(86, 5, "ESIIncludeNew: Requesting source '" << attr[i+1] << "'");

diff --git a/src/esi/Sequence.cc b/src/esi/Sequence.cc
index d080a324c8e5bb559cc4a3b594a52fb666aec647..75c794d8a59331477cbc4a6329e8f06fcd41928c 100644 (file)
--- a/src/esi/Sequence.cc
+++ b/src/esi/Sequence.cc
@@ -52,8 +52,19 @@ esiSequence::~esiSequence ()
     debugs(86, 5, "esiSequence::~esiSequence " << this);
 }
 
-esiSequence::esiSequence(esiTreeParentPtr aParent, bool incrementalFlag) : elements(), parent (aParent), mayFail_(true), failed (false), provideIncrementalData (incrementalFlag), processing (false), processingResult (ESI_PROCESS_COMPLETE), nextElementToProcess_ (0)
-{}
+esiSequence::esiSequence(esiTreeParentPtr aParent, bool incrementalFlag) :
+        elements(),
+        processedcount(0),
+        parent(aParent),
+        mayFail_(true),
+        failed(false),
+        provideIncrementalData(incrementalFlag),
+        processing(false),
+        processingResult(ESI_PROCESS_COMPLETE),
+        nextElementToProcess_(0)
+{
+    memset(&flags, 0, sizeof(flags));
+}
 
 size_t
 esiSequence::nextElementToProcess() const
@@ -329,11 +340,17 @@ esiSequence::fail (ESIElement *source, char const *anError)
     parent = NULL;
 }
 
-esiSequence::esiSequence(esiSequence const &old)
-        : processedcount (0), mayFail_(old.mayFail_), failed (old.failed), provideIncrementalData (old.provideIncrementalData), processing (false), nextElementToProcess_ (0)
+esiSequence::esiSequence(esiSequence const &old) :
+        processedcount(0),
+        parent(NULL),
+        mayFail_(old.mayFail_),
+        failed(old.failed),
+        provideIncrementalData(old.provideIncrementalData),
+        processing(false),
+        processingResult(ESI_PROCESS_COMPLETE),
+        nextElementToProcess_(0)
 {
     flags.dovars = old.flags.dovars;
-    parent = NULL;
 }
 
 void

diff --git a/src/esi/VarState.cc b/src/esi/VarState.cc
index 13282295bb7b99b0d0ef917ed9f4973148853eb8..f43e5b0de2b5693ba9057bbf240e03e1bc3b1dfb 100644 (file)
--- a/src/esi/VarState.cc
+++ b/src/esi/VarState.cc
@@ -291,9 +291,12 @@ ESIVariableQuery::~ESIVariableQuery()
     safe_free (query_string);
 }
 
-ESIVarState::ESIVarState (HttpHeader const *aHeader, char const *uri)
-        : output (NULL), hdr(hoReply)
+ESIVarState::ESIVarState(HttpHeader const *aHeader, char const *uri) :
+        output(NULL),
+        hdr(hoReply)
 {
+    memset(&flags, 0, sizeof(flags));
+
     /* TODO: only grab the needed headers */
     /* Note that as we pass these through to included requests, we
      * cannot trim them */