Fix warning in MicroLock initialization

Summary:The `init()` function uses the previous value of `lock_`, but that is
uninitialized and the compiler can issue a warning about it. It is
also potentially undefined behavior because it is not guaranteed that
the address of `lock_` is taken before `init()` (in which case the it
would be just an indeterminate value).

Since it is not very useful to initialize only one slot and leave the
others uninitialized, we can just have a single `init()` that
zero-initializes all the slots.

Reviewed By: dcolascione

Differential Revision: D3042629

fb-gh-sync-id: de1633b02eb1c891e310f2d5d2cfc5376cd41d5f
shipit-source-id: de1633b02eb1c891e310f2d5d2cfc5376cd41d5f

diff --git a/folly/MicroLock.h b/folly/MicroLock.h
index 159e3341c6dce3beb2ad2881193e4afeefb34f60..8f0b09f7d2902a1591bae1d48ab77c716e665476 100644 (file)
--- a/folly/MicroLock.h
+++ b/folly/MicroLock.h
@@ -102,8 +102,8 @@ class MicroLockCore {
  public:
   inline void unlock(unsigned slot);
   inline void unlock() { unlock(0); }
-  inline void init(unsigned slot) { lock_ &= ~(3U << (2 * slot)); }
-  inline void init() { init(0); }
+  // Initializes all the slots.
+  inline void init() { lock_ = 0; }
 };
 
 inline detail::Futex<>* MicroLockCore::word() const {

diff --git a/folly/test/SmallLocksTest.cpp b/folly/test/SmallLocksTest.cpp
index f33e023166a729281e61faf92cc381da85ea5778..7ddd29a070a163ca0870433c5c36617d4b1ee5cf 100644 (file)
--- a/folly/test/SmallLocksTest.cpp
+++ b/folly/test/SmallLocksTest.cpp
@@ -211,7 +211,7 @@ struct SimpleBarrier {
 };
 }
 
-static void runMicroLockTest() {
+TEST(SmallLocks, MicroLock) {
   volatile uint64_t counters[4] = {0, 0, 0, 0};
   std::vector<std::thread> threads;
   static const unsigned nrThreads = 20;
@@ -225,10 +225,7 @@ static void runMicroLockTest() {
   struct {
     uint8_t a;
     volatile uint8_t b;
-    union {
-      MicroLock alock;
-      uint8_t c;
-    };
+    MicroLock alock;
     volatile uint8_t d;
   } x;
 
@@ -237,7 +234,7 @@ static void runMicroLockTest() {
 
   x.a = 'a';
   x.b = origB;
-  x.c = 0;
+  x.alock.init();
   x.d = origD;
 
   // This thread touches other parts of the host word to show that
@@ -282,17 +279,16 @@ static void runMicroLockTest() {
 
   EXPECT_EQ(x.a, 'a');
   EXPECT_EQ(x.b, (uint8_t)(origB + iterPerThread / 2));
-  EXPECT_EQ(x.c, 0);
   EXPECT_EQ(x.d, (uint8_t)(origD + iterPerThread / 2));
   for (unsigned i = 0; i < 4; ++i) {
     EXPECT_EQ(counters[i], ((uint64_t)nrThreads * iterPerThread) / 4);
   }
 }
 
-TEST(SmallLocks, MicroLock) { runMicroLockTest(); }
 TEST(SmallLocks, MicroLockTryLock) {
   MicroLock lock;
   lock.init();
   EXPECT_TRUE(lock.try_lock());
   EXPECT_FALSE(lock.try_lock());
+  lock.unlock();
 }