9.2.x: latest clang-analyzer fixes

iocore/cache/Cache.cc

@@ -38,6 +38,8 @@
 #include "tscore/hugepages.h"
 
 #include <atomic>
+#include <numeric>
+#include <vector>
 
 constexpr ts::VersionNumber CACHE_DB_VERSION(CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION);
 
@@ -2873,10 +2875,8 @@ cplist_reconfigure()
       // else the size is greater...
       /* search the cp_list */
 
-      int *sorted_vols = new int[gndisks];
-      for (int i = 0; i < gndisks; i++) {
-        sorted_vols[i] = i;
-      }
+      std::vector<int> sorted_vols(gndisks);
+      std::iota(sorted_vols.begin(), sorted_vols.end(), 0);
       for (int i = 0; i < gndisks - 1; i++) {
         int smallest     = sorted_vols[i];
         int smallest_ndx = i;
@@ -2936,8 +2936,6 @@ cplist_reconfigure()
         size_to_alloc = size_in_blocks - cp->size;
       }
 
-      delete[] sorted_vols;
-
       if (size_to_alloc) {
         if (create_volume(volume_number, size_to_alloc, cp->scheme, cp)) {
           return -1;

iocore/net/QUICNetVConnection.cc

@@ -344,7 +344,7 @@ QUICNetVConnection::acceptEvent(int event, Event *e)
 
   MUTEX_TRY_LOCK(lock, h->mutex, t);
   if (!lock.is_locked()) {
-    if (event == EVENT_NONE) {
+    if (event == EVENT_NONE || e == nullptr) {
       t->schedule_in(this, HRTIME_MSECONDS(net_retry_delay));
       return EVENT_DONE;
     } else {

iocore/net/UnixNet.cc

@@ -23,6 +23,10 @@
 
 #include "P_Net.h"
 
+#if HAVE_EVENTFD
+#include <sys/eventfd.h>
+#endif
+
 using namespace std::literals;
 
 std::atomic<uint32_t> NetHandler::additional_accepts{0};
@@ -223,8 +227,8 @@ static void
 net_signal_hook_callback(EThread *thread)
 {
 #if HAVE_EVENTFD
-  uint64_t counter;
-  ATS_UNUSED_RETURN(read(thread->evfd, &counter, sizeof(uint64_t)));
+  eventfd_t counter;
+  ATS_UNUSED_RETURN(eventfd_read(thread->evfd, &counter));
 #elif TS_USE_PORT
 /* Nothing to drain or do */
 #else

iocore/net/UnixNetVConnection.cc

@@ -386,7 +386,7 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
       nh->write_ready_list.remove(vc);
     }
 
-    int err, ret;
+    int err{0}, ret{0};
 
     if (vc->get_context() == NET_VCONNECTION_OUT) {
       ret = vc->sslStartHandShake(SSL_EVENT_CLIENT, err);

mgmt/ProcessManager.cc

@@ -344,6 +344,7 @@ ProcessManager::initLMConnection()
 
   if ((local_manager_sockfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
     Fatal("Unable to create socket '%s': %s", sockpath.c_str(), strerror(errno));
+    return;
   }
 
   if (fcntl(local_manager_sockfd, F_SETFD, FD_CLOEXEC) < 0) {

plugins/experimental/memcache/tsmemcache.cc

@@ -238,7 +238,7 @@ MC::add_binary_header(uint16_t err, uint8_t hdr_len, uint16_t key_len, uint32_t
 int
 MC::write_binary_error(protocol_binary_response_status err, int swallow)
 {
-  const char *errstr = "Unknown error";
+  const char *errstr{nullptr};
   switch (err) {
   case PROTOCOL_BINARY_RESPONSE_ENOMEM:
     errstr = "Out of memory";

plugins/experimental/ssl_session_reuse/src/Config.h

@@ -88,6 +88,7 @@ class Config
   virtual ~Config();
 
   bool loadConfigOnChange();
+  bool checkConfigChange();
 
   static const int cCheckDivisor = 5;
 
@@ -100,6 +101,7 @@ class Config
   std::mutex m_yconfigLock;
   bool m_noConfig;
   bool m_alreadyLoaded;
+  bool m_loading = false;
   time_t m_lastCheck;
   time_t m_lastmtime;
 };

plugins/experimental/ssl_session_reuse/src/config.cc

@@ -23,6 +23,7 @@
  */
 
 #include <cstring>
+#include <cerrno>
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -46,58 +47,86 @@ Config::Config()
 
 Config::~Config() = default;
 
+namespace
+{
 bool
-Config::loadConfig(const std::string &filename)
+readConfig(const std::string &filename, std::map<std::string, std::string> &config)
 {
-  if (m_alreadyLoaded) {
-    return true;
+  int fd = open(filename.c_str(), O_RDONLY);
+  if (fd < 0) {
+    return false;
   }
-
-  bool success = false;
-
-  m_filename = filename;
-
-  int fd = (this->m_filename.length() > 0 ? open(m_filename.c_str(), O_RDONLY) : ts::NO_FD);
   struct stat info;
-  if (fd > 0 && 0 == fstat(fd, &info)) {
-    size_t n = info.st_size;
-    std::string config_data;
-    config_data.resize(n);
-    if (read(fd, const_cast<char *>(config_data.data()), n) != static_cast<int>(n)) {
+  if (fstat(fd, &info) != 0 || info.st_size < 0) {
+    close(fd);
+    return false;
+  }
+
+  std::string config_data(static_cast<size_t>(info.st_size), '\0');
+  size_t offset = 0;
+  while (offset < config_data.size()) {
+    ssize_t n = read(fd, config_data.data() + offset, config_data.size() - offset);
+    if (n < 0 && errno == EINTR) {
+      continue;
+    }
+    if (n <= 0) {
       close(fd);
-      return success;
+      return false;
     }
+    offset += static_cast<size_t>(n);
+  }
+  close(fd);
 
-    ts::TextView content(config_data);
-    while (content) {
-      ts::TextView line = content.take_prefix_at('\n');
-      if (line.empty() || '#' == *line) {
-        continue;
-      }
-      line.ltrim_if(&isspace);
-      ts::TextView field = line.take_prefix_at('=');
-      std::string field_name;
-      std::string value;
-      if (!field.empty()) {
-        field_name.assign(field.data(), field.size());
-      }
-      if (!line.empty()) {
-        value.assign(line.data(), line.size());
-      }
-      TSDebug(PLUGIN, "%.*s=%.*s", static_cast<int>(field_name.size()), field_name.c_str(), static_cast<int>(value.size()),
-              value.c_str());
-      if (!field_name.empty()) {
-        m_config[field_name] = value;
-      }
+  ts::TextView content(config_data);
+  while (content) {
+    ts::TextView line = content.take_prefix_at('\n');
+    if (line.empty() || '#' == *line) {
+      continue;
+    }
+    line.ltrim_if(&isspace);
+    ts::TextView field = line.take_prefix_at('=');
+    std::string field_name;
+    std::string value;
+    if (!field.empty()) {
+      field_name.assign(field.data(), field.size());
     }
+    if (!line.empty()) {
+      value.assign(line.data(), line.size());
+    }
+    TSDebug(PLUGIN, "%.*s=%.*s", static_cast<int>(field_name.size()), field_name.c_str(), static_cast<int>(value.size()),
+            value.c_str());
+    if (!field_name.empty()) {
+      config[field_name] = value;
+    }
+  }
+  return true;
+}
+} // namespace
 
-    close(fd);
+bool
+Config::loadConfig(const std::string &filename)
+{
+  {
+    std::lock_guard<std::mutex> lock(m_yconfigLock);
+    if (m_alreadyLoaded || m_loading) {
+      return m_alreadyLoaded;
+    }
+    m_filename = filename;
+    m_loading  = true;
+  }
+
+  // Parse without the lock, then publish the complete map under the lock.
+  // Readers see the previous map (empty during the first load), never a partial one.
+  std::map<std::string, std::string> config;
+  bool success = readConfig(filename, config);
 
+  std::lock_guard<std::mutex> lock(m_yconfigLock);
+  m_loading = false;
+  if (success) {
+    m_config.swap(config);
     m_noConfig      = false;
-    success         = true;
     m_alreadyLoaded = true;
   }
-
   return success;
 }
 
@@ -108,7 +137,9 @@ Config::setLastConfigChange()
   time_t oldLastmtime = m_lastmtime;
 
   memset(&s, 0, sizeof(s));
-  stat(m_filename.c_str(), &s);
+  if (stat(m_filename.c_str(), &s) != 0) {
+    return false;
+  }
 
   m_lastmtime = s.st_mtime;
 
@@ -120,10 +151,17 @@ Config::setLastConfigChange()
 
 bool
 Config::configHasChanged()
+{
+  std::lock_guard<std::mutex> lock(m_yconfigLock);
+  return !m_loading && checkConfigChange();
+}
+
+bool
+Config::checkConfigChange()
 {
   time_t checkTime = time(nullptr) / cCheckDivisor;
 
-  if (0 == m_lastmtime || m_lastCheck != checkTime) {
+  if (m_lastCheck != checkTime) {
     m_lastCheck = checkTime;
     return setLastConfigChange();
   }
@@ -133,33 +171,32 @@ Config::configHasChanged()
 bool
 Config::loadConfigOnChange()
 {
-  if (configHasChanged()) {
-    // loadConfig will check this, and if it hasn't been set it'll just bail.
+  std::string filename;
+  {
+    std::lock_guard<std::mutex> lock(m_yconfigLock);
+    if (m_loading || m_filename.empty()) {
+      return true;
+    }
+    if (!checkConfigChange()) {
+      return true;
+    }
     m_alreadyLoaded = false;
-    return loadConfig(m_filename);
+    filename        = m_filename;
   }
-
-  return true;
+  return loadConfig(filename);
 }
 
 bool
 Config::getValue(const std::string &category, const std::string &key, std::string &value)
 {
-  if (!m_noConfig) {
-    m_yconfigLock.lock();
-
-    if (loadConfigOnChange()) {
-      // convert to category.key= value.
-      std::string keyname                             = category + "." + key;
-      std::map<std::string, std::string>::iterator it = m_config.find(keyname);
-
-      // we have to use find so we don't overwrite defaults when we don't find anything.
-      if (m_config.end() != it) {
+  if (loadConfigOnChange()) {
+    std::lock_guard<std::mutex> lock(m_yconfigLock);
+    if (!m_noConfig) {
+      auto it = m_config.find(category + "." + key);
+      if (it != m_config.end()) {
         value = it->second;
       }
     }
-    m_yconfigLock.unlock();
   }
-
   return !value.empty();
 }

plugins/experimental/ssl_session_reuse/src/ssl_init.cc

@@ -106,7 +106,7 @@ get_redis_auth_key(char *retKeyBuff, int buffSize)
   if (ssl_param.redis_auth_key_file.length()) {
     int fd = open(ssl_param.redis_auth_key_file.c_str(), O_RDONLY);
     struct stat info;
-    if (0 == fstat(fd, &info)) {
+    if (fd >= 0 && 0 == fstat(fd, &info)) {
       size_t n = info.st_size;
       std::string key_data;
       key_data.resize(n);
@@ -115,9 +115,14 @@ get_redis_auth_key(char *retKeyBuff, int buffSize)
       while (read_len > 1 && key_data[read_len - 1] == '\n') {
         --read_len;
       }
-      memset(retKeyBuff, 0, buffSize);
-      strncpy(retKeyBuff, key_data.c_str(), read_len);
-      retval = key_data.length();
+      if (read_len > 0 && read_len < buffSize && static_cast<size_t>(read_len) <= key_data.length()) {
+        memset(retKeyBuff, 0, buffSize);
+        strncpy(retKeyBuff, key_data.c_str(), read_len);
+        retval = read_len;
+      }
+    }
+    if (fd >= 0) {
+      close(fd);
     }
   } else {
     TSError("Can not get redis auth key.");

plugins/experimental/system_stats/system_stats.c

@@ -178,6 +178,10 @@ setBondingStat(TSMutex stat_creation_mutex, const char *interface)
 
   snprintf(&infdir[0], sizeof(infdir), "%s/%s", NET_STATS_DIR, interface);
   DIR *localdir = opendir(infdir);
+  if (localdir == NULL) {
+    TSError("%s: Unable to open %s", DEBUG_TAG, infdir);
+    return;
+  }
 
   while ((dent = readdir(localdir)) != NULL) {
     if (((strncmp(SLAVE, dent->d_name, strlen(SLAVE)) == 0) || (strncmp(LOWER, dent->d_name, strlen(LOWER)) == 0)) &&