You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2015/02/23 18:52:21 UTC

trafficserver git commit: TS-3347: fix assertions that have side-effects

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


TS-3347: fix assertions that have side-effects

Coverity CID #1242347
Coverity CID #1242346
Coverity CID #1242344
Coverity CID #1242343
Coverity CID #1242341
Coverity CID #1242342


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

Branch: refs/heads/master
Commit: 0143ca195270fb17f003b05cf457570aa5a807ba
Parents: d08db5d
Author: James Peach <jp...@apache.org>
Authored: Tue Jan 27 10:03:30 2015 -0800
Committer: James Peach <jp...@apache.org>
Committed: Mon Feb 23 09:39:17 2015 -0800

----------------------------------------------------------------------
 iocore/cache/Cache.cc       |  2 +-
 iocore/eventsystem/I_Lock.h |  2 +-
 mgmt/Alarms.cc              | 14 +++++---------
 mgmt/cluster/ClusterCom.cc  |  6 ++----
 proxy/PluginVC.cc           | 29 ++++++++++++++---------------
 proxy/PluginVC.h            |  2 +-
 6 files changed, 24 insertions(+), 31 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0143ca19/iocore/cache/Cache.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc
index 8ac7946..5d429db 100644
--- a/iocore/cache/Cache.cc
+++ b/iocore/cache/Cache.cc
@@ -2832,7 +2832,7 @@ CacheVC::removeEvent(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */)
     if (!f.remove_aborted_writers) {
       if (vol->open_write(this, true, 1)) {
         // writer  exists
-        ink_assert(od = vol->open_read(&key));
+        ink_release_assert(od = vol->open_read(&key));
         od->dont_update_directory = 1;
         od = NULL;
       } else {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0143ca19/iocore/eventsystem/I_Lock.h
----------------------------------------------------------------------
diff --git a/iocore/eventsystem/I_Lock.h b/iocore/eventsystem/I_Lock.h
index bb3d2f3..d733e07 100644
--- a/iocore/eventsystem/I_Lock.h
+++ b/iocore/eventsystem/I_Lock.h
@@ -305,7 +305,7 @@ Mutex_trylock(
 #endif //DEBUG
       return false;
     }
-    ink_assert(m->thread_holding = t);
+    m->thread_holding = t;
 #ifdef DEBUG
     m->file = afile;
     m->line = aline;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0143ca19/mgmt/Alarms.cc
----------------------------------------------------------------------
diff --git a/mgmt/Alarms.cc b/mgmt/Alarms.cc
index aed1537..91f008f 100644
--- a/mgmt/Alarms.cc
+++ b/mgmt/Alarms.cc
@@ -297,17 +297,13 @@ Alarms::signalAlarm(alarm_t a, const char *desc, const char *ip)
     p++;
   if (*p == '\n')
     *p = '\0';
-  char *new_desc;
-  const size_t new_desc_size = sizeof(char) * (strlen(desc) + strlen(my_ctime_str) + 4);
-  ink_assert(new_desc = (char *) alloca(new_desc_size));
-  snprintf(new_desc, new_desc_size, "[%s] %s", my_ctime_str, desc);
-  desc = new_desc;
+
+  const size_t sz = sizeof(char) * (strlen(desc) + strlen(my_ctime_str) + 4);
   ats_free(atmp->description);
-  const size_t atmp_desc_size = sizeof(char) * (strlen(desc) + 1);
-  atmp->description = (char *)ats_malloc(atmp_desc_size);
-  ink_strlcpy(atmp->description, desc, atmp_desc_size);
-  ink_mutex_release(&mutex);
+  atmp->description = (char *)ats_malloc(sz);
+  snprintf(atmp->description, sz, "[%s] %s", my_ctime_str, desc);
 
+  ink_mutex_release(&mutex);
 
   for (entry = ink_hash_table_iterator_first(cblist, &iterator_state);
        entry != NULL; entry = ink_hash_table_iterator_next(cblist, &iterator_state)) {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0143ca19/mgmt/cluster/ClusterCom.cc
----------------------------------------------------------------------
diff --git a/mgmt/cluster/ClusterCom.cc b/mgmt/cluster/ClusterCom.cc
index 80e212e..a273a28 100644
--- a/mgmt/cluster/ClusterCom.cc
+++ b/mgmt/cluster/ClusterCom.cc
@@ -842,7 +842,7 @@ ClusterCom::handleMultiCastMessage(char *message)
   /* Have we see this guy before? */
   ink_mutex_acquire(&(mutex));  /* Grab cluster lock to access hash table */
   if (ink_hash_table_lookup(peers, (InkHashTableKey) ip, &hash_value) == 0) {
-    ink_assert((p = (ClusterPeerInfo *)ats_malloc(sizeof(ClusterPeerInfo))));
+    p = (ClusterPeerInfo *)ats_malloc(sizeof(ClusterPeerInfo));
     p->inet_address = inet_addr(ip);
     p->num_virt_addrs = 0;
 
@@ -1000,9 +1000,7 @@ ClusterCom::handleMultiCastStatPacket(char *last, ClusterPeerInfo * peer)
           rec->data.rec_string = NULL;
         } else if (!(strcmp(tmp_msg_val, "NULL") == 0)) {
           ats_free(rec->data.rec_string);
-          int rec_string_size = strlen(tmp_msg_val) + 1;
-          ink_assert((rec->data.rec_string = (RecString)ats_malloc(rec_string_size)));
-          ink_strlcpy(rec->data.rec_string, tmp_msg_val, rec_string_size);
+          rec->data.rec_string = (RecString)ats_strdup(tmp_msg_val);
         }
         break;
       }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0143ca19/proxy/PluginVC.cc
----------------------------------------------------------------------
diff --git a/proxy/PluginVC.cc b/proxy/PluginVC.cc
index 82f5e8e..2bed3f1 100644
--- a/proxy/PluginVC.cc
+++ b/proxy/PluginVC.cc
@@ -188,10 +188,10 @@ PluginVC::main_handler(int event, void *data)
   reentrancy_count++;
 
   if (call_event == active_event) {
-    process_timeout(call_event, VC_EVENT_ACTIVE_TIMEOUT, &active_event);
+    process_timeout(&active_event, VC_EVENT_ACTIVE_TIMEOUT);
   } else if (call_event == inactive_event) {
     if (inactive_timeout_at && inactive_timeout_at < ink_get_hrtime()) {
-      process_timeout(call_event, VC_EVENT_INACTIVITY_TIMEOUT, &inactive_event);
+      process_timeout(&inactive_event, VC_EVENT_INACTIVITY_TIMEOUT);
       call_event->cancel();
     }
   } else {
@@ -739,11 +739,11 @@ PluginVC::process_close()
   core_obj->attempt_delete();
 }
 
-// void PluginVC::process_timeout(Event* e, int event_to_send, Event** our_eptr)
+// void PluginVC::process_timeout(Event** e, int event_to_send, Event** our_eptr)
 //
 //   Handles sending timeout event to the VConnection.  e is the event we got
-//     which indicats the timeout.  event_to_send is the event to the
-//     vc user.  Our_eptr is a pointer our event either inactive_event,
+//     which indicates the timeout.  event_to_send is the event to the
+//     vc user.  e is a pointer to either inactive_event,
 //     or active_event.  If we successfully send the timeout to vc user,
 //     we clear the pointer, otherwise we reschedule it.
 //
@@ -751,29 +751,28 @@ PluginVC::process_close()
 //      touch any state after making the call back
 //
 void
-PluginVC::process_timeout(Event * e, int event_to_send, Event ** our_eptr)
+PluginVC::process_timeout(Event ** e, int event_to_send)
 {
-
-  ink_assert(e = *our_eptr);
+  ink_assert(*e == inactive_event || *e == active_event);
 
   if (read_state.vio.op == VIO::READ && !read_state.shutdown && read_state.vio.ntodo() > 0) {
-    MUTEX_TRY_LOCK(lock, read_state.vio.mutex, e->ethread);
+    MUTEX_TRY_LOCK(lock, read_state.vio.mutex, (*e)->ethread);
     if (!lock.is_locked()) {
-      e->schedule_in(PVC_LOCK_RETRY_TIME);
+      (*e)->schedule_in(PVC_LOCK_RETRY_TIME);
       return;
     }
-    *our_eptr = NULL;
+    *e = NULL;
     read_state.vio._cont->handleEvent(event_to_send, &read_state.vio);
   } else if (write_state.vio.op == VIO::WRITE && !write_state.shutdown && write_state.vio.ntodo() > 0) {
-    MUTEX_TRY_LOCK(lock, write_state.vio.mutex, e->ethread);
+    MUTEX_TRY_LOCK(lock, write_state.vio.mutex, (*e)->ethread);
     if (!lock.is_locked()) {
-      e->schedule_in(PVC_LOCK_RETRY_TIME);
+      (*e)->schedule_in(PVC_LOCK_RETRY_TIME);
       return;
     }
-    *our_eptr = NULL;
+    *e = NULL;
     write_state.vio._cont->handleEvent(event_to_send, &write_state.vio);
   } else {
-    *our_eptr = NULL;
+    *e = NULL;
   }
 }
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0143ca19/proxy/PluginVC.h
----------------------------------------------------------------------
diff --git a/proxy/PluginVC.h b/proxy/PluginVC.h
index ecf1da8..90ff7e8 100644
--- a/proxy/PluginVC.h
+++ b/proxy/PluginVC.h
@@ -135,7 +135,7 @@ private:
   void process_read_side(bool);
   void process_write_side(bool);
   void process_close();
-  void process_timeout(Event * e, int event_to_send, Event ** our_eptr);
+  void process_timeout(Event ** e, int event_to_send);
 
   void setup_event_cb(ink_hrtime in, Event ** e_ptr);