Review comments

Author: Michael Kipper

ext/semian/resource.c

@@ -12,6 +12,9 @@ check_quota_arg(VALUE quota);
 static int
 check_tickets_arg(VALUE tickets);
 
+static int
+check_min_tickets_arg(VALUE min_tickets);
+
 static long
 check_permissions_arg(VALUE permissions);
 
@@ -209,7 +212,7 @@ semian_resource_initialize(VALUE self, VALUE id, VALUE tickets, VALUE quota, VAL
   long c_permissions = check_permissions_arg(permissions);
   const char *c_id_str = check_id_arg(id);
   double c_timeout = check_default_timeout_arg(default_timeout);
-  int c_min_tickets = check_tickets_arg(min_tickets);
+  int c_min_tickets = check_min_tickets_arg(min_tickets);
 
   // Build semian resource structure
   semian_resource_t *res = NULL;
@@ -303,6 +306,32 @@ check_tickets_arg(VALUE tickets)
   return c_tickets;
 }
 
+static int
+check_min_tickets_arg(VALUE min_tickets)
+{
+  int retval = -1;
+
+  switch (rb_type(min_tickets)) {
+  case T_NIL:
+  case T_UNDEF:
+    return -1;
+  case T_FLOAT:
+    rb_warn("semian min_tickets value %f is a float, converting to fixnum", RFLOAT_VALUE(min_tickets));
+    retval = (int) RFLOAT_VALUE(min_tickets);
+    break;
+  case T_FIXNUM:
+    retval = FIX2LONG(min_tickets); break;
+  default:
+    retval = -1; break;
+  }
+
+  if (retval <= 0 || retval > system_max_semaphore_count) {
+    rb_raise(rb_eArgError, "max_tickets must be in range [1,%d)", system_max_semaphore_count);
+  }
+
+  return retval;
+}
+
 static const char*
 check_id_arg(VALUE id)
 {

ext/semian/tickets.c

@@ -80,7 +80,7 @@ max(const int a, const int b)
 }
 
 static int
-calculate_quota_tickets (int sem_id, double quota, int min_tickets)
+calculate_quota_tickets(int sem_id, double quota, int min_tickets)
 {
   int workers = get_sem_val(sem_id, SI_SEM_REGISTERED_WORKERS);
   int tickets = (int) ceil(workers * quota);

test/resource_test.rb

@@ -506,6 +506,14 @@ def test_min_tickets
     assert_equal(8, resource.tickets)
   end
 
+  def test_min_tickets_float
+    expected_warning = /semian min_tickets value 2\.000000 is a float, converting to fixnum/
+    with_fake_std_error(warn_message: expected_warning) do
+      id = Time.now.strftime('%H:%M:%S.%N')
+      Semian::Resource.new(id, quota: 0.49, timeout: 0.1, min_tickets: 2.0)
+    end
+  end
+
   def test_min_tickets_nil
     id = Time.now.strftime('%H:%M:%S.%N')
     resource = Semian::Resource.new(id, quota: 0.49, timeout: 0.1, min_tickets: nil)
@@ -522,16 +530,9 @@ def test_min_tickets_nil
 
   def test_min_tickets_zero
     id = Time.now.strftime('%H:%M:%S.%N')
-    resource = Semian::Resource.new(id, quota: 0.49, timeout: 0.1, min_tickets: 0)
-    assert_equal(1, resource.tickets)
-    fork_workers(resource: id, count: 1, quota: 0.49, min_tickets: 0, timeout: 0.1, wait_for_timeout: true)
-    assert_equal(1, resource.tickets)
-    fork_workers(resource: id, count: 1, quota: 0.49, min_tickets: 0, timeout: 0.1, wait_for_timeout: true)
-    assert_equal(2, resource.tickets)
-    fork_workers(resource: id, count: 1, quota: 0.49, min_tickets: 0, timeout: 0.1, wait_for_timeout: true)
-    assert_equal(2, resource.tickets)
-    fork_workers(resource: id, count: 12, quota: 0.49, min_tickets: 0, timeout: 0.1, wait_for_timeout: true)
-    assert_equal(8, resource.tickets)
+    assert_raises ArgumentError do
+      Semian::Resource.new(id, quota: 0.49, timeout: 0.1, min_tickets: 0)
+    end
   end
 
   def test_min_tickets_negative
@@ -541,6 +542,13 @@ def test_min_tickets_negative
     end
   end
 
+  def test_min_tickets_out_of_range
+    id = Time.now.strftime('%H:%M:%S.%N')
+    assert_raises ArgumentError do
+      Semian::Resource.new(id, quota: 0.49, timeout: 0.1, min_tickets: 32768)
+    end
+  end
+
   def create_resource(*args)
     @resources ||= []
     resource = Semian::Resource.new(*args)