From 1377ed413818f92848c459b66b751db2960ef8bd Mon Sep 17 00:00:00 2001
From: Alan Liddell <aliddell@chanzuckerberg.com>
Date: Tue, 9 Jul 2024 16:04:23 -0400
Subject: [PATCH] Respond to PR comments.

---
 src/common/thread.pool.cpp | 16 ++++++++--------
 src/common/thread.pool.hh  |  2 +-
 src/common/utilities.cpp   |  6 +-----
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/src/common/thread.pool.cpp b/src/common/thread.pool.cpp
index 79a57eb5..534b0849 100644
--- a/src/common/thread.pool.cpp
+++ b/src/common/thread.pool.cpp
@@ -4,15 +4,14 @@
 namespace zarr = acquire::sink::zarr;
 namespace common = zarr::common;
 
-common::ThreadPool::ThreadPool(size_t n_threads,
+common::ThreadPool::ThreadPool(unsigned int n_threads,
                                std::function<void(const std::string&)> err)
   : error_handler_{ err }
   , is_accepting_jobs_{ true }
 {
+    const unsigned int one = 1;
     n_threads = std::clamp(
-      n_threads,
-      (size_t)1,
-      (size_t)std::max(std::thread::hardware_concurrency(), (unsigned)1));
+      n_threads, one, std::max(std::thread::hardware_concurrency(), one));
 
     for (auto i = 0; i < n_threads; ++i) {
         threads_.emplace_back([this] { thread_worker_(); });
@@ -32,11 +31,12 @@ common::ThreadPool::~ThreadPool() noexcept
 void
 common::ThreadPool::push_to_job_queue(JobT&& job)
 {
-    std::unique_lock lock(jobs_mutex_);
-    CHECK(is_accepting_jobs_);
+    {
+        std::unique_lock lock(jobs_mutex_);
+        CHECK(is_accepting_jobs_);
 
-    jobs_.push(std::move(job));
-    lock.unlock();
+        jobs_.push(std::move(job));
+    }
 
     cv_.notify_one();
 }
diff --git a/src/common/thread.pool.hh b/src/common/thread.pool.hh
index ac29ae9d..a5e24c01 100644
--- a/src/common/thread.pool.hh
+++ b/src/common/thread.pool.hh
@@ -20,7 +20,7 @@ struct ThreadPool final
     // std::string& argument to the error handler is a diagnostic message from
     // the failing job and is logged to the error stream by the Zarr driver when
     // the next call to `append()` is made.
-    ThreadPool(size_t n_threads, std::function<void(const std::string&)> err);
+    ThreadPool(unsigned int n_threads, std::function<void(const std::string&)> err);
     ~ThreadPool() noexcept;
 
     void push_to_job_queue(JobT&& job);
diff --git a/src/common/utilities.cpp b/src/common/utilities.cpp
index 90d8e70d..63b98f94 100644
--- a/src/common/utilities.cpp
+++ b/src/common/utilities.cpp
@@ -201,15 +201,11 @@ common::split_uri(const std::string& uri)
     auto end = uri.find_first_of(delim);
 
     std::vector<std::string> out;
-    while (end <= std::string::npos) {
+    while (end != std::string::npos) {
         if (end > begin) {
             out.emplace_back(uri.substr(begin, end - begin));
         }
 
-        if (end == std::string::npos) {
-            break;
-        }
-
         begin = end + 1;
         end = uri.find_first_of(delim, begin);
     }