work on review comments

base_c/include/embb/base/c/mutex.h

@@ -163,6 +163,7 @@ void embb_mutex_destroy(
 /**
  * Initializes a spinlock
  *
+ * \pre \c spinlock is uninitialized
  * \post \c spinlock is initialized
  * \return EMBB_SUCCESS if spinlock could be initialized \n
  *         EMBB_ERROR otherwise
@@ -206,8 +207,9 @@ int embb_spin_try_lock(
   embb_spinlock_t* spinlock,
   /**< [IN/OUT] Pointer to spinlock */
   unsigned int max_number_spins
-  /**< [IN] Number of attempts the locking operation is repeated if
-   *        unsuccessful */
+  /**< [IN] Maximal count of spins to perform, trying to acquire lock. Note that
+   * passing 0 here results in not trying to obtain the lock at all.
+   */
   );
 
 /**

base_c/src/mutex.c

@@ -117,9 +117,9 @@ void embb_mutex_destroy(embb_mutex_t* mutex) {
 #endif /* EMBB_PLATFORM_THREADING_POSIXTHREADS */
 
 int embb_spin_init(embb_spinlock_t* spinlock) {
-  // for now, just assign the internal struct value... in the future,
-  // we will have an atomic init function.
-  spinlock->atomic_spin_variable_.internal_variable = 0;
+  // For now, store the initial value. In the future will use atomic init
+  // function (as soon as available).
+  embb_atomic_store_int(&spinlock->atomic_spin_variable_, 0);
 }
 
 int embb_spin_lock(embb_spinlock_t* spinlock) {
@@ -128,9 +128,6 @@ int embb_spin_lock(embb_spinlock_t* spinlock) {
   // try to swap the
   while (0 == embb_atomic_compare_and_swap_int(
     &spinlock->atomic_spin_variable_, &expected, 1)) {
-    // mtapi has a debug variable, counting spins... think about that...
-    // embb_atomic_fetch_and_add_int(&embb_mtapi_spinlock_spins, 1);
-
     // reset expected, as CAS might change it...
     expected = 0;
   }
@@ -139,16 +136,15 @@ int embb_spin_lock(embb_spinlock_t* spinlock) {
 
 int embb_spin_try_lock(embb_spinlock_t* spinlock,
   unsigned int max_number_spins) {
+  if (max_number_spins == 0)
+    return EMBB_BUSY;
+
   int expected = 0;
-  unsigned int spin_count = max_number_spins;
   while (0 == embb_atomic_compare_and_swap_int(
     &spinlock->atomic_spin_variable_,
     &expected, 1)) {
-    // mtapi has a debug variable, counting spins... think about that...
-    // embb_atomic_fetch_and_add_int(&embb_mtapi_spinlock_spins, 1);
-
-    spin_count--;
-    if (0 == spin_count) {
+    max_number_spins--;
+    if (0 == max_number_spins) {
       return EMBB_BUSY;
     }
     expected = 0;

base_cpp/include/embb/base/internal/mutex-inl.h

@@ -114,6 +114,8 @@ bool UniqueLock<Mutex>::OwnsLock() const {
   return locked_;
 }
 
+
+
 } // namespace base
 } // namespace embb
 

base_cpp/include/embb/base/mutex.h

@@ -180,18 +180,14 @@ class Spinlock {
    *
    * \notthreadsafe
    */
-  Spinlock() {
-    embb_spin_init(&spinlock_);
-  }
+  Spinlock();
 
   /**
    * Destructs a spinlock.
    *
    * \notthreadsafe
    */
-  ~Spinlock() {
-    embb_spin_destroy(&spinlock_);
-  }
+  ~Spinlock();
 
   /**
    * Waits until the spinlock can be locked and locks it.
@@ -201,15 +197,7 @@ class Spinlock {
    * \threadsafe
    * \see TryLock(), Unlock()
    */
-  void Lock() {
-    int status = embb_spin_lock(&spinlock_);
-
-    // Currently, embb_spin_lock will always return EMBB_SUCCESS. However,
-    // This might change.
-    if (status != EMBB_SUCCESS) {
-      EMBB_THROW(ErrorException, "Error in embb_spin_lock");
-    }
-  }
+  void Lock();
 
   /**
    * Tries to lock the spinlock for \c number_spins times and returns.
@@ -222,19 +210,11 @@ class Spinlock {
    */
   bool TryLock(
     unsigned int number_spins = 1
-    /**< [IN] Maximal spins to perform, trying to acquire lock */
-    ) {
-    int status = embb_spin_try_lock(&spinlock_, number_spins);
-
-    if (status == EMBB_BUSY){
-      return false;
-    }
-    else if (status != EMBB_SUCCESS) {
-      EMBB_THROW(ErrorException, "Error in embb_spin_try_lock");
-    }
-
-    return true;
-  }
+    /**< [IN] Maximal count of spins to perform, trying to acquire lock.
+     * Note that passing 0 here results in not trying to obtain the lock at all.
+     * Default parameter is 1.
+     */
+    );
 
   /**
    * Unlocks the spinlock.
@@ -244,13 +224,7 @@ class Spinlock {
    * \threadsafe
    * \see Lock(), TryLock()
    */
-  void Unlock() {
-    int status = embb_spin_unlock(&spinlock_);
-
-    if (status != EMBB_SUCCESS) {
-      EMBB_THROW(ErrorException, "Error in embb_spin_unlock");
-    }
-  }
+  void Unlock();
 
  private:
   /**

base_cpp/src/mutex.cc

@@ -60,5 +60,43 @@ Mutex::Mutex() : MutexBase(EMBB_MUTEX_PLAIN) {
 RecursiveMutex::RecursiveMutex() : MutexBase(EMBB_MUTEX_RECURSIVE) {
 }
 
+Spinlock::Spinlock() {
+  embb_spin_init(&spinlock_);
+}
+
+Spinlock::~Spinlock() {
+  embb_spin_destroy(&spinlock_);
+}
+
+void Spinlock::Lock() {
+  int status = embb_spin_lock(&spinlock_);
+
+  // Currently, embb_spin_lock will always return EMBB_SUCCESS. However,
+  // This might change.
+  if (status != EMBB_SUCCESS) {
+    EMBB_THROW(ErrorException, "Error in embb_spin_lock");
+  }
+}
+
+bool Spinlock::TryLock(unsigned int number_spins) {
+  int status = embb_spin_try_lock(&spinlock_, number_spins);
+
+  if (status == EMBB_BUSY){
+    return false;
+  }
+  else if (status != EMBB_SUCCESS) {
+    EMBB_THROW(ErrorException, "Error in embb_spin_try_lock");
+  }
+
+  return true;
+}
+
+void Spinlock::Unlock() {
+  int status = embb_spin_unlock(&spinlock_);
+
+  if (status != EMBB_SUCCESS) {
+    EMBB_THROW(ErrorException, "Error in embb_spin_unlock");
+  }
+}
 } // namespace base
 } // namespace embb