From c899191ad587f8718b0161601df395e88ded1dce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Botond=20D=C3=A9nes?= <bdenes@scylladb.com>
Date: Fri, 14 Dec 2018 15:03:55 +0200
Subject: [PATCH] reader_concurrency_semaphore: use the correct types in the
 constructor

Previously there was a type mismatch for `count` and `memory`, between
the actual type used to store them in the class (signed) and the type
of the parameters in the constructor (unsigned).
Although negative numbers are completely valid for these members,
initializing them to negative numbers don't make sense, this is why they
used unsigned types in the constructor. This restriction can backfire
however when someone intends to give these parameters the maximum
possible value, which, when interpreted as a signed value will be `-1`.
What's worse the caller might not even be aware of this unsigned->signed
conversion and be very suprised when they find out.
So to prevent surprises, expose the real type of these members, trusting
the clients of knowing what they are doing.

Also add a `no_limits` constructor, so clients don't have to make sure
they don't overflow internal types.

(cherry picked from commit e1d8237e6b2702cd3622fb628022f9ee7515dffb)
---
 reader_concurrency_semaphore.hh | 13 +++++++++++--
 tests/querier_cache.cc          |  2 +-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/reader_concurrency_semaphore.hh b/reader_concurrency_semaphore.hh
index 5b721963e4..a364117201 100644
--- a/reader_concurrency_semaphore.hh
+++ b/reader_concurrency_semaphore.hh
@@ -179,8 +179,10 @@ private:
         signal(resources(0, static_cast<ssize_t>(memory)));
     }
 public:
-    reader_concurrency_semaphore(unsigned count,
-            size_t memory,
+    struct no_limits { };
+
+    reader_concurrency_semaphore(int count,
+            ssize_t memory,
             size_t max_queue_length = std::numeric_limits<size_t>::max(),
             std::function<std::exception_ptr()> raise_queue_overloaded_exception = default_make_queue_overloaded_exception)
         : _resources(count, memory)
@@ -188,6 +190,13 @@ public:
         , _make_queue_overloaded_exception(raise_queue_overloaded_exception) {
     }
 
+    /// Create a semaphore with practically unlimited count and memory.
+    ///
+    /// And conversely, no queue limit either.
+    explicit reader_concurrency_semaphore(no_limits)
+        : reader_concurrency_semaphore(std::numeric_limits<int>::max(), std::numeric_limits<ssize_t>::max()) {
+    }
+
     reader_concurrency_semaphore(const reader_concurrency_semaphore&) = delete;
     reader_concurrency_semaphore& operator=(const reader_concurrency_semaphore&) = delete;
 
diff --git a/tests/querier_cache.cc b/tests/querier_cache.cc
index f7a3e06b59..5a9e04a9d1 100644
--- a/tests/querier_cache.cc
+++ b/tests/querier_cache.cc
@@ -158,7 +158,7 @@ public:
     };
 
     test_querier_cache(const noncopyable_function<sstring(size_t)>& external_make_value, std::chrono::seconds entry_ttl = 24h, size_t cache_size = 100000)
-        : _sem(std::numeric_limits<unsigned>::max(), std::numeric_limits<size_t>::max())
+        : _sem(reader_concurrency_semaphore::no_limits{})
         , _cache(_sem, cache_size, entry_ttl)
         , _mutations(make_mutations(_s, external_make_value))
         , _mutation_source([this] (schema_ptr, const dht::partition_range& range) {