trafficserver git commit: TS-3287: fix memory management in the escalate plugin

[jp...@apache.org]

Repository: trafficserver
Updated Branches:
  refs/heads/master 0143ca195 -> 8c71ba112


TS-3287: fix memory management in the escalate plugin

Keeping duplicate pointers in the escalation map will double-free.
Convert this code to just copy the retry target to each entry.

This also fixes Coverity CID #1200022.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/8c71ba11
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/8c71ba11
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/8c71ba11

Branch: refs/heads/master
Commit: 8c71ba11240fe6cd62cf2a5d818616b5adee33b1
Parents: 0143ca1
Author: James Peach <jp...@apache.org>
Authored: Sun Feb 1 11:03:13 2015 -0800
Committer: James Peach <jp...@apache.org>
Committed: Mon Feb 23 09:53:31 2015 -0800

----------------------------------------------------------------------
 plugins/experimental/escalate/escalate.cc | 28 +++++++++++---------------
 1 file changed, 12 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/8c71ba11/plugins/experimental/escalate/escalate.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/escalate/escalate.cc b/plugins/experimental/escalate/escalate.cc
index 12c655a..e4cc829 100644
--- a/plugins/experimental/escalate/escalate.cc
+++ b/plugins/experimental/escalate/escalate.cc
@@ -53,7 +53,7 @@ struct EscalationState
     std::string target;
   };
 
-  typedef std::map<unsigned, RetryInfo*> StatusMapType;
+  typedef std::map<unsigned, RetryInfo> StatusMapType;
 
   EscalationState()
   {
@@ -63,9 +63,6 @@ struct EscalationState
 
   ~EscalationState()
   {
-    for (StatusMapType::iterator iter = status_map.begin(); iter != status_map.end(); ++iter) {
-      delete(iter->second);
-    }
     TSContDestroy(cont);
   }
 
@@ -82,7 +79,7 @@ EscalateResponse(TSCont cont, TSEvent event, void* edata)
 {
   TSHttpTxn txn = (TSHttpTxn)edata;
   EscalationState* es = static_cast<EscalationState*>(TSContDataGet(cont));
-  EscalationState::StatusMapType::iterator entry;
+  EscalationState::StatusMapType::const_iterator entry;
   TSMBuffer mbuf;
   TSMLoc hdrp, url;
   TSHttpStatus status;
@@ -113,15 +110,15 @@ EscalateResponse(TSCont cont, TSEvent event, void* edata)
   }
 
   TSDebug(PLUGIN_NAME, "Found an entry for HTTP status %u", (unsigned)status);
-  if (EscalationState::RETRY_URL == entry->second->type) {
-    url_str = TSstrdup(entry->second->target.c_str());
-    url_len = entry->second->target.size();
+  if (EscalationState::RETRY_URL == entry->second.type) {
+    url_str = TSstrdup(entry->second.target.c_str());
+    url_len = entry->second.target.size();
     TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
-  } else if (EscalationState::RETRY_HOST == entry->second->type) {
+  } else if (EscalationState::RETRY_HOST == entry->second.type) {
     if (TS_SUCCESS == TSHttpTxnClientReqGet(txn, &mbuf, &hdrp)) {
       if (TS_SUCCESS == TSHttpHdrUrlGet(mbuf, hdrp, &url)) {
         // Update the request URL with the new Host to try.
-        TSUrlHostSet(mbuf, url, entry->second->target.c_str(), entry->second->target.size());
+        TSUrlHostSet(mbuf, url, entry->second.target.c_str(), entry->second.target.size());
         url_str = TSUrlStringGet(mbuf, url, &url_len);
         TSDebug(PLUGIN_NAME, "Setting new Host: to %.*s", url_len, url_str);
       }
@@ -170,14 +167,14 @@ TSRemapNewInstance(int argc, char* argv[], void** instance, char* errbuf, int er
     ++sep; // Skip over the ':' (which is now \0)
 
     // OK, we have a valid status/URL pair.
-    EscalationState::RetryInfo* info = new EscalationState::RetryInfo();
+    EscalationState::RetryInfo info;
 
-    info->target = sep;
-    if (std::string::npos != info->target.find('/')) {
-      info->type = EscalationState::RETRY_URL;
+    info.target = sep;
+    if (std::string::npos != info.target.find('/')) {
+      info.type = EscalationState::RETRY_URL;
       TSDebug(PLUGIN_NAME, "Creating Redirect rule with URL = %s", sep);
     } else {
-      info->type = EscalationState::RETRY_HOST;
+      info.type = EscalationState::RETRY_HOST;
       TSDebug(PLUGIN_NAME, "Creating Redirect rule with Host = %s", sep);
     }
 
@@ -186,7 +183,6 @@ TSRemapNewInstance(int argc, char* argv[], void** instance, char* errbuf, int er
 
       if (status < 100 || status > 599) {
         snprintf(errbuf, errbuf_size, "invalid status code: %.*s", (int)std::distance(argv[i], sep), argv[i]);
-        delete info;
         goto fail;
       }