Address compiler warnings #10

2023-11-08 19:53:12 -06:00
parent a7209184c8
commit f2c1f64f02
30 changed files with 737 additions and 615 deletions
--- a/src/comm/curl/curl_comm.cpp
+++ b/src/comm/curl/curl_comm.cpp
@@ -43,32 +43,36 @@ const curl_comm::write_callback curl_comm::write_headers =
      auto &headers = *reinterpret_cast<http_headers *>(outstream);
      const auto header = std::string(buffer, size * nitems);
      const auto parts = utils::string::split(header, ':');
-      if (parts.size() > 1u) {
-        auto data = header.substr(parts[0u].size() + 1u);
+      if (parts.size() > 1U) {
+        auto data = header.substr(parts[0U].size() + 1U);
        utils::string::left_trim(data);
        utils::string::right_trim(data, '\r');
        utils::string::right_trim(data, '\n');
        utils::string::right_trim(data, '\r');
-        headers[utils::string::to_lower(parts[0u])] = data;
+        headers[utils::string::to_lower(parts[0U])] = data;
      }
      return size * nitems;
    });

-curl_comm::curl_comm(host_config hc)
-    : host_config_(std::move(hc)), s3_config_(std::nullopt) {}
+curl_comm::curl_comm(host_config cfg)
+    : host_config_(std::move(cfg)), s3_config_(std::nullopt) {}

-curl_comm::curl_comm(s3_config s3)
-    : host_config_(std::nullopt), s3_config_(std::move(s3)) {}
+curl_comm::curl_comm(s3_config cfg)
+    : host_config_(std::nullopt), s3_config_(std::move(cfg)) {}

 auto curl_comm::construct_url(CURL *curl, const std::string &relative_path,
-                              const host_config &hc) -> std::string {
-  auto custom_port =
-      (((hc.protocol == "http") && (hc.api_port == 80U || hc.api_port == 0U)) ||
-       ((hc.protocol == "https") && (hc.api_port == 443U || hc.api_port == 0U)))
-          ? ""
-          : ":" + std::to_string(hc.api_port);
-  auto url = hc.protocol + "://" +
-             utils::string::trim_copy(hc.host_name_or_ip) + custom_port;
+                              const host_config &cfg) -> std::string {
+  static constexpr const auto http = 80U;
+  static constexpr const auto https = 443U;
+
+  auto custom_port = (((cfg.protocol == "http") &&
+                       (cfg.api_port == http || cfg.api_port == 0U)) ||
+                      ((cfg.protocol == "https") &&
+                       (cfg.api_port == https || cfg.api_port == 0U)))
+                         ? ""
+                         : ":" + std::to_string(cfg.api_port);
+  auto url = cfg.protocol + "://" +
+             utils::string::trim_copy(cfg.host_name_or_ip) + custom_port;

  static const auto complete_url = [](const std::string &current_path,
                                      const std::string &parent_path,
@@ -80,40 +84,40 @@ auto curl_comm::construct_url(CURL *curl, const std::string &relative_path,
    return final_url;
  };

-  auto path = utils::path::combine("/", {hc.path});
+  auto path = utils::path::combine("/", {cfg.path});
  return relative_path.empty()
-             ? complete_url(path, hc.path, url)
+             ? complete_url(path, cfg.path, url)
             : complete_url(utils::path::combine(
                                path, {url_encode(curl, relative_path, true)}),
                            relative_path, url);
 }

-auto curl_comm::create_host_config(const s3_config &config,
-                                   bool use_s3_path_style) -> host_config {
-  host_config hc{};
-  hc.api_password = config.secret_key;
-  hc.api_user = config.access_key;
+auto curl_comm::create_host_config(const s3_config &cfg, bool use_s3_path_style)
+    -> host_config {
+  host_config host_cfg{};
+  host_cfg.api_password = cfg.secret_key;
+  host_cfg.api_user = cfg.access_key;

-  auto pos = config.url.find(':');
-  hc.host_name_or_ip = config.url.substr(pos + 3U);
-  if (config.use_region_in_url && not config.region.empty()) {
-    auto parts = utils::string::split(hc.host_name_or_ip, '.', false);
+  auto pos = cfg.url.find(':');
+  host_cfg.host_name_or_ip = cfg.url.substr(pos + 3U);
+  if (cfg.use_region_in_url && not cfg.region.empty()) {
+    auto parts = utils::string::split(host_cfg.host_name_or_ip, '.', false);
    if (parts.size() > 1U) {
-      parts.insert(parts.begin() + 1U, config.region);
-      hc.host_name_or_ip = utils::string::join(parts, '.');
+      parts.insert(parts.begin() + 1U, cfg.region);
+      host_cfg.host_name_or_ip = utils::string::join(parts, '.');
    }
  }

  if (not use_s3_path_style) {
-    hc.host_name_or_ip = config.bucket + '.' + hc.host_name_or_ip;
+    host_cfg.host_name_or_ip = cfg.bucket + '.' + host_cfg.host_name_or_ip;
  }

-  hc.protocol = config.url.substr(0U, pos);
+  host_cfg.protocol = cfg.url.substr(0U, pos);
  if (use_s3_path_style) {
-    hc.path = '/' + config.bucket;
+    host_cfg.path = '/' + cfg.bucket;
  }

-  return hc;
+  return host_cfg;
 }

 void curl_comm::enable_s3_path_style(bool enable) {
@@ -126,7 +130,7 @@ auto curl_comm::make_request(const curl::requests::http_delete &del,
  return make_request(
      s3_config_.has_value()
          ? create_host_config(s3_config_.value(), use_s3_path_style_)
-          : host_config_.value(),
+          : host_config_.value_or(host_config{}),
      del, response_code, stop_requested);
 }

@@ -136,7 +140,7 @@ auto curl_comm::make_request(const curl::requests::http_get &get,
  return make_request(
      s3_config_.has_value()
          ? create_host_config(s3_config_.value(), use_s3_path_style_)
-          : host_config_.value(),
+          : host_config_.value_or(host_config{}),
      get, response_code, stop_requested);
 }

@@ -146,7 +150,7 @@ auto curl_comm::make_request(const curl::requests::http_head &head,
  return make_request(
      s3_config_.has_value()
          ? create_host_config(s3_config_.value(), use_s3_path_style_)
-          : host_config_.value(),
+          : host_config_.value_or(host_config{}),
      head, response_code, stop_requested);
 }

@@ -156,7 +160,7 @@ auto curl_comm::make_request(const curl::requests::http_put_file &put_file,
  return make_request(
      s3_config_.has_value()
          ? create_host_config(s3_config_.value(), use_s3_path_style_)
-          : host_config_.value(),
+          : host_config_.value_or(host_config{}),
      put_file, response_code, stop_requested);
 }

--- a/src/comm/curl/multi_request.cpp
+++ b/src/comm/curl/multi_request.cpp
@@ -38,6 +38,8 @@ multi_request::~multi_request() {
 }

 void multi_request::get_result(CURLcode &curl_code, long &http_code) {
+  static constexpr const auto timeout_ms = 100;
+
  curl_code = CURLcode::CURLE_ABORTED_BY_CALLBACK;
  http_code = -1;

@@ -45,8 +47,8 @@ void multi_request::get_result(CURLcode &curl_code, long &http_code) {
  int running_handles = 0;
  curl_multi_perform(multi_handle_, &running_handles);
  while (not error && (running_handles > 0) && not stop_requested_) {
-    int ignored;
-    curl_multi_wait(multi_handle_, nullptr, 0, 100, &ignored);
+    int ignored{};
+    curl_multi_wait(multi_handle_, nullptr, 0, timeout_ms, &ignored);

    const auto ret = curl_multi_perform(multi_handle_, &running_handles);
    error = (ret != CURLM_CALL_MULTI_PERFORM) && (ret != CURLM_OK);
@@ -56,7 +58,7 @@ void multi_request::get_result(CURLcode &curl_code, long &http_code) {
    int remaining_messages = 0;
    auto *multi_result =
        curl_multi_info_read(multi_handle_, &remaining_messages);
-    if (multi_result && (multi_result->msg == CURLMSG_DONE)) {
+    if ((multi_result != nullptr) && (multi_result->msg == CURLMSG_DONE)) {
      curl_easy_getinfo(multi_result->easy_handle, CURLINFO_RESPONSE_CODE,
                        &http_code);
      curl_code = multi_result->data.result;
--- a/src/comm/packet/client_pool.cpp
+++ b/src/comm/packet/client_pool.cpp
@@ -30,23 +30,23 @@ void client_pool::pool::execute(
    std::uint64_t thread_id, const worker_callback &worker,
    const worker_complete_callback &worker_complete) {
  const auto index = thread_id % pool_queues_.size();
-  auto wi = std::make_shared<work_item>(worker, worker_complete);
+  auto job = std::make_shared<work_item>(worker, worker_complete);
  auto &pool_queue = pool_queues_[index];

  unique_mutex_lock queue_lock(pool_queue->mutex);
-  pool_queue->queue.emplace_back(wi);
+  pool_queue->queue.emplace_back(job);
  pool_queue->notify.notify_all();
  queue_lock.unlock();
 }

 client_pool::pool::pool(std::uint8_t pool_size) {
  event_system::instance().raise<service_started>("client_pool");
-  thread_index_ = 0u;
-  for (std::uint8_t i = 0u; i < pool_size; i++) {
+
+  for (std::uint8_t i = 0U; i < pool_size; i++) {
    pool_queues_.emplace_back(std::make_unique<work_queue>());
  }

-  for (std::size_t i = 0u; i < pool_queues_.size(); i++) {
+  for (std::size_t i = 0U; i < pool_queues_.size(); i++) {
    pool_threads_.emplace_back([this]() {
      const auto thread_index = thread_index_++;

@@ -88,12 +88,12 @@ client_pool::pool::pool(std::uint8_t pool_size) {

      queue_lock.lock();
      while (not queue.empty()) {
-        auto wi = queue.front();
+        auto job = queue.front();
        queue.pop_front();
        queue_notify.notify_all();
        queue_lock.unlock();

-        wi->work_complete(utils::from_api_error(api_error::download_stopped));
+        job->work_complete(utils::from_api_error(api_error::download_stopped));

        queue_lock.lock();
      }
@@ -108,7 +108,7 @@ void client_pool::pool::shutdown() {
  shutdown_ = true;

  for (auto &pool_queue : pool_queues_) {
-    unique_mutex_lock l(pool_queue->mutex);
+    mutex_lock lock(pool_queue->mutex);
    pool_queue->notify.notify_all();
  }

@@ -148,8 +148,8 @@ void client_pool::shutdown() {
    unique_mutex_lock pool_lock(pool_mutex_);
    if (not shutdown_) {
      shutdown_ = true;
-      for (auto &kv : pool_lookup_) {
-        kv.second->shutdown();
+      for (auto &pool : pool_lookup_) {
+        pool.second->shutdown();
      }
      pool_lookup_.clear();
    }
--- a/src/comm/s3/s3_comm.cpp
+++ b/src/comm/s3/s3_comm.cpp
@@ -46,6 +46,8 @@ s3_comm::s3_comm(const app_config &config)

  // TODO make configurable
  const auto enable_path_style =
+      utils::string::begins_with(s3_config_.url,
+                                 "https://gateway.storjshare.io") ||
      utils::string::begins_with(s3_config_.url, "http://localhost") ||
      utils::string::begins_with(s3_config_.url, "https://localhost") ||
      utils::string::begins_with(s3_config_.url, "http://127.0.0.1") ||
--- a/src/comm/s3/s3_requests_curl.cpp
+++ b/src/comm/s3/s3_requests_curl.cpp
@@ -37,6 +37,7 @@ get_object_list(i_http_comm &client, const s3_config &config,
                std::optional<std::string> delimiter = std::nullopt,
                std::optional<std::string> prefix = std::nullopt) -> bool {
  curl::requests::http_get get{};
+  get.allow_timeout = true;
  get.aws_service = "aws:amz:" + config.region + ":s3";
  get.path = '/';
  get.query["list-type"] = "2";
@@ -62,6 +63,7 @@ auto create_directory_object_request_impl(i_http_comm &client,
                                          long &response_code) -> bool {
  try {
    curl::requests::http_put_file put_file{};
+    put_file.allow_timeout = true;
    put_file.aws_service = "aws:amz:" + config.region + ":s3";
    put_file.file_name =
        *(utils::string::split(object_name, '/', false).end() - 1U);
@@ -86,11 +88,12 @@ auto delete_object_request_impl(i_http_comm &client, const s3_config &config,
      return false;
    }

-    if (response_code == 404) {
+    if (response_code == http_error_codes::not_found) {
      return true;
    }

    curl::requests::http_delete del{};
+    del.allow_timeout = true;
    del.aws_service = "aws:amz:" + config.region + ":s3";
    del.path = '/' + object_name;

@@ -109,6 +112,7 @@ auto head_object_request_impl(i_http_comm &client, const s3_config &config,
    -> bool {
  try {
    curl::requests::http_head head{};
+    head.allow_timeout = true;
    head.aws_service = "aws:amz:" + config.region + ":s3";
    head.path = '/' + object_name;
    head.response_headers = http_headers{};
@@ -118,7 +122,7 @@ auto head_object_request_impl(i_http_comm &client, const s3_config &config,
      return false;
    }

-    if (response_code == 200) {
+    if (response_code == http_error_codes::ok) {
      result.from_headers(head.response_headers.value());
    }

@@ -139,7 +143,7 @@ auto list_directories_request_impl(i_http_comm &client, const s3_config &config,
      return false;
    }

-    if (response_code != 200) {
+    if (response_code != http_error_codes::ok) {
      return false;
    }

@@ -151,7 +155,7 @@ auto list_directories_request_impl(i_http_comm &client, const s3_config &config,

    auto node_list = doc.select_nodes("/ListBucketResult/Contents");
    for (const auto &node : node_list) {
-      auto object_name =
+      const auto *object_name =
          node.node().select_node("Key").node().text().as_string();
      if (utils::string::ends_with(object_name, "/")) {
        api_file directory{};
@@ -186,7 +190,7 @@ auto list_files_request_impl(
      return false;
    }

-    if (response_code != 200) {
+    if (response_code != http_error_codes::ok) {
      return false;
    }

@@ -203,7 +207,7 @@ auto list_files_request_impl(
      if (not utils::string::ends_with(object_name, "/")) {
        api_file file{};
        object_name = get_name(
-            *(utils::string::split(object_name, '/', false).end() - 1u),
+            *(utils::string::split(object_name, '/', false).end() - 1U),
            object_name);
        file.api_path = utils::path::create_api_path(object_name);
        file.api_parent = utils::path::get_parent_api_path(file.api_path);
@@ -242,7 +246,7 @@ auto list_objects_in_directory_request_impl(
      return false;
    }

-    if (response_code != 200) {
+    if (response_code != http_error_codes::ok) {
      return false;
    }

@@ -255,14 +259,15 @@ auto list_objects_in_directory_request_impl(
    const auto add_directory_item =
        [&](bool directory, const std::string &name,
            std::function<std::uint64_t(const directory_item &)> get_size) {
-          directory_item di{};
-          di.api_path =
+          directory_item dir_item{};
+          dir_item.api_path =
              utils::path::create_api_path(utils::path::combine("/", {name}));
-          di.api_parent = utils::path::get_parent_api_path(di.api_path);
-          di.directory = directory;
-          di.size = get_size(di);
-          meta_provider(di);
-          result.emplace_back(std::move(di));
+          dir_item.api_parent =
+              utils::path::get_parent_api_path(dir_item.api_path);
+          dir_item.directory = directory;
+          dir_item.size = get_size(dir_item);
+          meta_provider(dir_item);
+          result.emplace_back(std::move(dir_item));
        };

    auto node_list =
@@ -275,7 +280,7 @@ auto list_objects_in_directory_request_impl(

    node_list = doc.select_nodes("/ListBucketResult/Contents");
    for (const auto &node : node_list) {
-      auto child_object_name =
+      const auto *child_object_name =
          node.node().select_node("Key").node().text().as_string();
      if (child_object_name != prefix) {
        auto size = node.node().select_node("Size").node().text().as_ullong();
@@ -302,7 +307,7 @@ auto list_objects_request_impl(i_http_comm &client, const s3_config &config,
      return false;
    }

-    if (response_code != 200) {
+    if (response_code != http_error_codes::ok) {
      return false;
    }

@@ -314,16 +319,16 @@ auto list_objects_request_impl(i_http_comm &client, const s3_config &config,

    auto node_list = doc.select_nodes("/ListBucketResult/Contents");
    for (const auto &node : node_list) {
-      auto object_name =
+      const auto *object_name =
          node.node().select_node("Key").node().text().as_string();
      auto size = node.node().select_node("Size").node().text().as_ullong();
-      directory_item di{};
-      di.api_path = utils::path::create_api_path(object_name);
-      di.api_parent = utils::path::get_parent_api_path(di.api_path);
-      di.directory = utils::string::ends_with(object_name, "/");
-      di.size = di.directory ? 0U : size;
-      di.resolved = false;
-      result.emplace_back(std::move(di));
+      directory_item dir_item{};
+      dir_item.api_path = utils::path::create_api_path(object_name);
+      dir_item.api_parent = utils::path::get_parent_api_path(dir_item.api_path);
+      dir_item.directory = utils::string::ends_with(object_name, "/");
+      dir_item.size = dir_item.directory ? 0U : size;
+      dir_item.resolved = false;
+      result.emplace_back(std::move(dir_item));
    }

    return true;