You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2019/04/30 15:38:23 UTC

[trafficserver] branch master updated: cppcheck: Fix various issues found in iocore/eventsystem

This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 494a1ea  cppcheck: Fix various issues found in iocore/eventsystem
494a1ea is described below

commit 494a1ead9bc711f3baa8657b90ad7d3caa3f18c4
Author: Bill Chen <bi...@linkedin.com>
AuthorDate: Thu Apr 25 08:49:49 2019 -0700

    cppcheck: Fix various issues found in iocore/eventsystem
---
 iocore/eventsystem/IOBuffer.cc           |  4 +---
 iocore/eventsystem/I_Continuation.h      |  4 ++--
 iocore/eventsystem/I_EThread.h           |  1 +
 iocore/eventsystem/I_EventProcessor.h    |  2 +-
 iocore/eventsystem/I_Lock.h              |  2 +-
 iocore/eventsystem/I_MIOBufferWriter.h   |  2 +-
 iocore/eventsystem/I_VConnection.h       | 10 +++++-----
 iocore/eventsystem/I_VIO.h               |  2 +-
 iocore/eventsystem/P_Freer.h             | 11 +++++++----
 iocore/eventsystem/P_UnixSocketManager.h |  8 +++-----
 iocore/eventsystem/ProtectedQueue.cc     |  2 +-
 iocore/eventsystem/SocketManager.cc      |  4 +---
 iocore/eventsystem/UnixEThread.cc        |  8 ++++----
 13 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/iocore/eventsystem/IOBuffer.cc b/iocore/eventsystem/IOBuffer.cc
index 1fcd758..844a5af 100644
--- a/iocore/eventsystem/IOBuffer.cc
+++ b/iocore/eventsystem/IOBuffer.cc
@@ -46,8 +46,6 @@ int64_t max_iobuffer_size           = DEFAULT_BUFFER_SIZES - 1;
 void
 init_buffer_allocators(int iobuffer_advice)
 {
-  char *name;
-
   for (int i = 0; i < DEFAULT_BUFFER_SIZES; i++) {
     int64_t s = DEFAULT_BUFFER_BASE_SIZE * (((int64_t)1) << i);
     int64_t a = DEFAULT_BUFFER_ALIGNMENT;
@@ -56,7 +54,7 @@ init_buffer_allocators(int iobuffer_advice)
       a = s;
     }
 
-    name = new char[64];
+    auto name = new char[64];
     snprintf(name, 64, "ioBufAllocator[%d]", i);
     ioBufAllocator[i].re_init(name, s, n, a, iobuffer_advice);
   }
diff --git a/iocore/eventsystem/I_Continuation.h b/iocore/eventsystem/I_Continuation.h
index b4531a9..b34f78d 100644
--- a/iocore/eventsystem/I_Continuation.h
+++ b/iocore/eventsystem/I_Continuation.h
@@ -198,8 +198,8 @@ protected:
     @param amutex Lock to be set for this Continuation.
 
   */
-  Continuation(ProxyMutex *amutex = nullptr);
-  Continuation(Ptr<ProxyMutex> &amutex);
+  explicit Continuation(ProxyMutex *amutex = nullptr);
+  explicit Continuation(Ptr<ProxyMutex> &amutex);
 };
 
 /**
diff --git a/iocore/eventsystem/I_EThread.h b/iocore/eventsystem/I_EThread.h
index 7e86c8e..c70fb04 100644
--- a/iocore/eventsystem/I_EThread.h
+++ b/iocore/eventsystem/I_EThread.h
@@ -358,6 +358,7 @@ public:
   */
   class DefaultTailHandler : public LoopTailHandler
   {
+    // cppcheck-suppress noExplicitConstructor; allow implicit conversion
     DefaultTailHandler(ProtectedQueue &q) : _q(q) {}
 
     int
diff --git a/iocore/eventsystem/I_EventProcessor.h b/iocore/eventsystem/I_EventProcessor.h
index 031ca71..a15d406 100644
--- a/iocore/eventsystem/I_EventProcessor.h
+++ b/iocore/eventsystem/I_EventProcessor.h
@@ -391,7 +391,7 @@ private:
     EventProcessor *_evp;
 
   public:
-    ThreadInit(EventProcessor *evp) : _evp(evp) { SET_HANDLER(&self::init); }
+    explicit ThreadInit(EventProcessor *evp) : _evp(evp) { SET_HANDLER(&self::init); }
 
     int
     init(int /* event ATS_UNUSED */, Event *ev)
diff --git a/iocore/eventsystem/I_Lock.h b/iocore/eventsystem/I_Lock.h
index c061d26..b5bef2c 100644
--- a/iocore/eventsystem/I_Lock.h
+++ b/iocore/eventsystem/I_Lock.h
@@ -245,7 +245,7 @@ Mutex_trylock(
   Ptr<ProxyMutex> &m, EThread *t)
 {
   ink_assert(t != nullptr);
-  ink_assert(t == (EThread *)this_thread());
+  ink_assert(t == reinterpret_cast<EThread *>(this_thread()));
   if (m->thread_holding != t) {
     if (!ink_mutex_try_acquire(&m->the_mutex)) {
 #ifdef DEBUG
diff --git a/iocore/eventsystem/I_MIOBufferWriter.h b/iocore/eventsystem/I_MIOBufferWriter.h
index 639f461..8568816 100644
--- a/iocore/eventsystem/I_MIOBufferWriter.h
+++ b/iocore/eventsystem/I_MIOBufferWriter.h
@@ -42,7 +42,7 @@ class MIOBufferWriter : public ts::BufferWriter
   using self_type = MIOBufferWriter; ///< Self reference type.
 
 public:
-  MIOBufferWriter(MIOBuffer *miob) : _miob(miob) {}
+  explicit MIOBufferWriter(MIOBuffer *miob) : _miob(miob) {}
 
   self_type &write(const void *data_, size_t length) override;
 
diff --git a/iocore/eventsystem/I_VConnection.h b/iocore/eventsystem/I_VConnection.h
index 193c62b..80a7218 100644
--- a/iocore/eventsystem/I_VConnection.h
+++ b/iocore/eventsystem/I_VConnection.h
@@ -313,8 +313,8 @@ public:
   */
   virtual void do_io_shutdown(ShutdownHowTo_t howto) = 0;
 
-  VConnection(ProxyMutex *aMutex);
-  VConnection(Ptr<ProxyMutex> &aMutex);
+  explicit VConnection(ProxyMutex *aMutex);
+  explicit VConnection(Ptr<ProxyMutex> &aMutex);
 
   // Private
   // Set continuation on a given vio. The public interface
@@ -389,8 +389,8 @@ class AnnotatedVConnection : public VConnection
   using super_type = VConnection;
 
 public:
-  AnnotatedVConnection(ProxyMutex *aMutex) : super_type(aMutex){};
-  AnnotatedVConnection(Ptr<ProxyMutex> &aMutex) : super_type(aMutex){};
+  explicit AnnotatedVConnection(ProxyMutex *aMutex) : super_type(aMutex){};
+  explicit AnnotatedVConnection(Ptr<ProxyMutex> &aMutex) : super_type(aMutex){};
 
   void *
   get_user_arg(unsigned ix) const
@@ -440,5 +440,5 @@ struct DummyVConnection : public AnnotatedVConnection {
                 "cannot use default implementation");
   }
 
-  DummyVConnection(ProxyMutex *m) : AnnotatedVConnection(m) {}
+  explicit DummyVConnection(ProxyMutex *m) : AnnotatedVConnection(m) {}
 };
diff --git a/iocore/eventsystem/I_VIO.h b/iocore/eventsystem/I_VIO.h
index 40d8fa4..e31c5dd 100644
--- a/iocore/eventsystem/I_VIO.h
+++ b/iocore/eventsystem/I_VIO.h
@@ -139,7 +139,7 @@ public:
   */
   inkcoreapi void reenable_re();
 
-  VIO(int aop);
+  explicit VIO(int aop);
   VIO();
 
   enum {
diff --git a/iocore/eventsystem/P_Freer.h b/iocore/eventsystem/P_Freer.h
index 374af33..deb916d 100644
--- a/iocore/eventsystem/P_Freer.h
+++ b/iocore/eventsystem/P_Freer.h
@@ -44,7 +44,7 @@ public: // Needed by WinNT compiler (compiler bug)
     delete this;
     return EVENT_DONE;
   }
-  DeleterContinuation(C *ap) : Continuation(new_ProxyMutex()), p(ap) { SET_HANDLER(&DeleterContinuation::dieEvent); }
+  explicit DeleterContinuation(C *ap) : Continuation(new_ProxyMutex()), p(ap) { SET_HANDLER(&DeleterContinuation::dieEvent); }
 };
 
 // This can be useful for two things (or both):
@@ -73,7 +73,7 @@ public: // Needed by WinNT compiler (compiler bug)
     delete this;
     return EVENT_DONE;
   }
-  FreeCallContinuation(C *ap) : Continuation(nullptr), p(ap) { SET_HANDLER(&FreeCallContinuation::dieEvent); }
+  explicit FreeCallContinuation(C *ap) : Continuation(nullptr), p(ap) { SET_HANDLER(&FreeCallContinuation::dieEvent); }
 };
 
 template <class C>
@@ -99,7 +99,10 @@ struct FreerContinuation : public Continuation {
     return EVENT_DONE;
   }
 
-  FreerContinuation(void *ap) : Continuation(nullptr), p(ap) { SET_HANDLER((FreerContHandler)&FreerContinuation::dieEvent); }
+  explicit FreerContinuation(void *ap) : Continuation(nullptr), p(ap)
+  {
+    SET_HANDLER((FreerContHandler)&FreerContinuation::dieEvent);
+  }
 };
 
 TS_INLINE void
@@ -123,7 +126,7 @@ template <class C> struct DereferContinuation : public Continuation {
     return EVENT_DONE;
   }
 
-  DereferContinuation(C *ap) : Continuation(nullptr), p(ap) { SET_HANDLER(&DereferContinuation::dieEvent); }
+  explicit DereferContinuation(C *ap) : Continuation(nullptr), p(ap) { SET_HANDLER(&DereferContinuation::dieEvent); }
 };
 
 template <class C>
diff --git a/iocore/eventsystem/P_UnixSocketManager.h b/iocore/eventsystem/P_UnixSocketManager.h
index b90b2b1..dc5c0c9 100644
--- a/iocore/eventsystem/P_UnixSocketManager.h
+++ b/iocore/eventsystem/P_UnixSocketManager.h
@@ -114,14 +114,12 @@ SocketManager::vector_io(int fd, struct iovec *vector, size_t count, int read_re
 {
   const int max_iovecs_per_request = 16;
   int n;
-  int64_t r = 0;
+  int64_t r;
   int n_vec;
   int64_t bytes_xfered = 0;
-  int current_count;
-  int64_t current_request_bytes;
 
   for (n_vec = 0; n_vec < (int)count; n_vec += max_iovecs_per_request) {
-    current_count = std::min(max_iovecs_per_request, ((int)(count - n_vec)));
+    int current_count = std::min(max_iovecs_per_request, ((int)(count - n_vec)));
     do {
       // coverity[tainted_data_argument]
       r = read_request ? ::readv(fd, &vector[n_vec], current_count) : ::writev(fd, &vector[n_vec], current_count);
@@ -141,7 +139,7 @@ SocketManager::vector_io(int fd, struct iovec *vector, size_t count, int read_re
     }
 
     // Compute bytes in current vector
-    current_request_bytes = 0;
+    int64_t current_request_bytes = 0;
     for (n = n_vec; n < (n_vec + current_count); ++n) {
       current_request_bytes += vector[n].iov_len;
     }
diff --git a/iocore/eventsystem/ProtectedQueue.cc b/iocore/eventsystem/ProtectedQueue.cc
index bd434e7..e8741c7 100644
--- a/iocore/eventsystem/ProtectedQueue.cc
+++ b/iocore/eventsystem/ProtectedQueue.cc
@@ -93,7 +93,7 @@ ProtectedQueue::dequeue_timed(ink_hrtime cur_time, ink_hrtime timeout, bool slee
 void
 ProtectedQueue::dequeue_external()
 {
-  Event *e = (Event *)ink_atomiclist_popall(&al);
+  Event *e = static_cast<Event *>(ink_atomiclist_popall(&al));
   // invert the list, to preserve order
   SLL<Event, Event::Link_link> l, t;
   t.head = e;
diff --git a/iocore/eventsystem/SocketManager.cc b/iocore/eventsystem/SocketManager.cc
index 24bc38b..6dffbe1 100644
--- a/iocore/eventsystem/SocketManager.cc
+++ b/iocore/eventsystem/SocketManager.cc
@@ -66,10 +66,8 @@ accept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags)
 int
 SocketManager::accept4(int s, struct sockaddr *addr, socklen_t *addrlen, int flags)
 {
-  int fd;
-
   do {
-    fd = ::accept4(s, addr, addrlen, flags);
+    int fd = ::accept4(s, addr, addrlen, flags);
     if (likely(fd >= 0)) {
       return fd;
     }
diff --git a/iocore/eventsystem/UnixEThread.cc b/iocore/eventsystem/UnixEThread.cc
index a579322..03b31c3 100644
--- a/iocore/eventsystem/UnixEThread.cc
+++ b/iocore/eventsystem/UnixEThread.cc
@@ -193,16 +193,16 @@ EThread::execute_regular()
 {
   Event *e;
   Que(Event, link) NegativeQueue;
-  ink_hrtime next_time = 0;
-  ink_hrtime delta     = 0;    // time spent in the event loop
+  ink_hrtime next_time;
+  ink_hrtime delta;            // time spent in the event loop
   ink_hrtime loop_start_time;  // Time the loop started.
   ink_hrtime loop_finish_time; // Time at the end of the loop.
 
   // Track this so we can update on boundary crossing.
   EventMetrics *prev_metric = this->prev(metrics + (ink_get_hrtime_internal() / HRTIME_SECOND) % N_EVENT_METRICS);
 
-  int nq_count = 0;
-  int ev_count = 0;
+  int nq_count;
+  int ev_count;
 
   // A statically initialized instance we can use as a prototype for initializing other instances.
   static EventMetrics METRIC_INIT;