Fixed a snappy compression wrapper bug (passing wrong variable).

Change atomic_pointer.h to prefer a memory barrier based
implementation over a <cstdatomic> based implementation for
the following reasons:
(1) On a x86-32-bit gcc-4.4 build, <ctdatomic> was corrupting
    the AtomicPointer.
(2) On a x86-64-bit gcc build, a <ctstdatomic> based acquire-load
    takes ~15ns as opposed to the ~1ns for a memory-barrier
    based implementation.

Fixes issue 9 (corruption_test fails)
http://code.google.com/p/leveldb/issues/detail?id=9

Fixes issue 16 (CorruptionTest.MissingDescriptor fails)
http://code.google.com/p/leveldb/issues/detail?id=16



git-svn-id: https://leveldb.googlecode.com/svn/trunk@36 62dab493-f737-651d-591e-8d6aee1b9529

port/atomic_pointer.h

@@ -4,57 +4,31 @@
 
 // AtomicPointer provides storage for a lock-free pointer.
 // Platform-dependent implementation of AtomicPointer:
+// - If the platform provides a cheap barrier, we use it with raw pointers
 // - If cstdatomic is present (on newer versions of gcc, it is), we use
-//   a cstdatomic-based AtomicPointer
-// - If it is not, we define processor-dependent AtomicWord operations,
-//   and then use them to build AtomicPointer
-//
+//   a cstdatomic-based AtomicPointer.  However we prefer the memory
+//   barrier based version, because at least on a gcc 4.4 32-bit build
+//   on linux, we have encountered a buggy <cstdatomic>
+//   implementation.  Also, some <cstdatomic> implementations are much
+//   slower than a memory-barrier based implementation (~16ns for
+//   <cstdatomic> based acquire-load vs. ~1ns for a barrier based
+//   acquire-load).
 // This code is based on atomicops-internals-* in Google's perftools:
 // http://code.google.com/p/google-perftools/source/browse/#svn%2Ftrunk%2Fsrc%2Fbase
 
 #ifndef PORT_ATOMIC_POINTER_H_
 #define PORT_ATOMIC_POINTER_H_
 
+#include <stdint.h>
 #ifdef LEVELDB_CSTDATOMIC_PRESENT
-
-///////////////////////////////////////////////////////////////////////////////
-// WE HAVE <cstdatomic>
-// Use a <cstdatomic>-based AtomicPointer
-
 #include <cstdatomic>
-#include <stdint.h>
-
-namespace leveldb {
-namespace port {
-
-// Storage for a lock-free pointer
-class AtomicPointer {
- private:
-  std::atomic<void*> rep_;
- public:
-  AtomicPointer() { }
-  explicit AtomicPointer(void* v) : rep_(v) { }
-  inline void* Acquire_Load() const {
-    return rep_.load(std::memory_order_acquire);
-  }
-  inline void Release_Store(void* v) {
-    rep_.store(v, std::memory_order_release);
-  }
-  inline void* NoBarrier_Load() const {
-    return rep_.load(std::memory_order_relaxed);
-  }
-  inline void NoBarrier_Store(void* v) {
-    rep_.store(v, std::memory_order_relaxed);
-  }
-};
-
-} // namespace leveldb::port
-} // namespace leveldb
-
-#else
-///////////////////////////////////////////////////////////////////////////////
-// NO <cstdatomic>
-// The entire rest of this file covers that case
+#endif
+#ifdef OS_WIN
+#include <windows.h>
+#endif
+#ifdef OS_MACOSX
+#include <libkern/OSAtomic.h>
+#endif
 
 #if defined(_M_X64) || defined(__x86_64__)
 #define ARCH_CPU_X86_FAMILY 1
@@ -62,152 +36,100 @@ class AtomicPointer {
 #define ARCH_CPU_X86_FAMILY 1
 #elif defined(__ARMEL__)
 #define ARCH_CPU_ARM_FAMILY 1
-#else
-#warning Please add support for your architecture in atomicpointer.h
 #endif
 
 namespace leveldb {
 namespace port {
-namespace internal {
-
-// AtomicWord is a machine-sized pointer.
-typedef intptr_t AtomicWord;
-
-} // namespace leveldb::port::internal
-} // namespace leveldb::port
-} // namespace leveldb
 
-// Include our platform specific implementation.
-///////////////////////////////////////////////////////////////////////////////
+// Define MemoryBarrier() if available
 // Windows on x86
 #if defined(OS_WIN) && defined(COMPILER_MSVC) && defined(ARCH_CPU_X86_FAMILY)
-
-// void MemoryBarrier(void) macro is defined in windows.h:
+// windows.h already provides a MemoryBarrier(void) macro
 // http://msdn.microsoft.com/en-us/library/ms684208(v=vs.85).aspx
-// Including windows.h here; MemoryBarrier() gets used below.
-#include <windows.h>
-
-///////////////////////////////////////////////////////////////////////////////
-// Mac OS on x86
-#elif defined(OS_MACOSX) && defined(ARCH_CPU_X86_FAMILY)
-
-#include <libkern/OSAtomic.h>
-
-namespace leveldb {
-namespace port {
-namespace internal {
+#define LEVELDB_HAVE_MEMORY_BARRIER
 
+// Gcc on x86
+#elif defined(__GNUC__) && defined(ARCH_CPU_X86_FAMILY)
 inline void MemoryBarrier() {
-#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
   // See http://gcc.gnu.org/ml/gcc/2003-04/msg01180.html for a discussion on
   // this idiom. Also see http://en.wikipedia.org/wiki/Memory_ordering.
   __asm__ __volatile__("" : : : "memory");
-#else
-  OSMemoryBarrier();
-#endif
 }
+#define LEVELDB_HAVE_MEMORY_BARRIER
 
-} // namespace leveldb::port::internal
-} // namespace leveldb::port
-} // namespace leveldb
-
-///////////////////////////////////////////////////////////////////////////////
-// Any x86 CPU
-#elif defined(ARCH_CPU_X86_FAMILY)
-
-namespace leveldb {
-namespace port {
-namespace internal {
-
+// Mac OS
+#elif defined(OS_MACOSX)
 inline void MemoryBarrier() {
-  __asm__ __volatile__("" : : : "memory");
+  OSMemoryBarrier();
 }
+#define LEVELDB_HAVE_MEMORY_BARRIER
 
-} // namespace leveldb::port::internal
-} // namespace leveldb::port
-} // namespace leveldb
-
-#undef ATOMICOPS_COMPILER_BARRIER
-
-///////////////////////////////////////////////////////////////////////////////
 // ARM
 #elif defined(ARCH_CPU_ARM_FAMILY)
-
-namespace leveldb {
-namespace port {
-namespace internal {
-
 typedef void (*LinuxKernelMemoryBarrierFunc)(void);
 LinuxKernelMemoryBarrierFunc pLinuxKernelMemoryBarrier __attribute__((weak)) =
     (LinuxKernelMemoryBarrierFunc) 0xffff0fa0;
-
 inline void MemoryBarrier() {
   pLinuxKernelMemoryBarrier();
 }
+#define LEVELDB_HAVE_MEMORY_BARRIER
 
-} // namespace leveldb::port::internal
-} // namespace leveldb::port
-} // namespace leveldb
-
-#else
-#error "Atomic operations are not supported on your platform"
 #endif
 
-///////////////////////////////////////////////////////////////////////////////
-// Implementation of AtomicPointer based on MemoryBarriers above
-
-namespace leveldb {
-namespace port {
-namespace internal {
-
-// Atomic operations using per-system MemoryBarrier()s
-
-inline AtomicWord Acquire_Load(volatile const AtomicWord* ptr) {
-  AtomicWord value = *ptr;
-  MemoryBarrier();
-  return value;
-}
-
-inline void Release_Store(volatile AtomicWord* ptr, AtomicWord value) {
-  MemoryBarrier();
-  *ptr = value;
-}
-
-inline AtomicWord NoBarrier_Load(volatile const AtomicWord* ptr) {
-  return *ptr;
-}
-
-inline void NoBarrier_Store(volatile AtomicWord* ptr, AtomicWord value) {
-  *ptr = value;
-}
-
-} // namespace leveldb::port::internal
+// AtomicPointer built using platform-specific MemoryBarrier()
+#if defined(LEVELDB_HAVE_MEMORY_BARRIER)
+class AtomicPointer {
+ private:
+  void* rep_;
+ public:
+  AtomicPointer() { }
+  explicit AtomicPointer(void* p) : rep_(p) {}
+  inline void* NoBarrier_Load() const { return rep_; }
+  inline void NoBarrier_Store(void* v) { rep_ = v; }
+  inline void* Acquire_Load() const {
+    void* result = rep_;
+    MemoryBarrier();
+    return result;
+  }
+  inline void Release_Store(void* v) {
+    MemoryBarrier();
+    rep_ = v;
+  }
+};
 
-// AtomicPointer definition for systems without <cstdatomic>.
+// AtomicPointer based on <cstdatomic>
+#elif defined(LEVELDB_CSTDATOMIC_PRESENT)
 class AtomicPointer {
  private:
-  typedef internal::AtomicWord Rep;
-  Rep rep_;
+  std::atomic<void*> rep_;
  public:
   AtomicPointer() { }
-  explicit AtomicPointer(void* p) : rep_(reinterpret_cast<Rep>(p)) {}
+  explicit AtomicPointer(void* v) : rep_(v) { }
   inline void* Acquire_Load() const {
-    return reinterpret_cast<void*>(internal::Acquire_Load(&rep_));
+    return rep_.load(std::memory_order_acquire);
   }
   inline void Release_Store(void* v) {
-    internal::Release_Store(&rep_, reinterpret_cast<Rep>(v));
+    rep_.store(v, std::memory_order_release);
   }
   inline void* NoBarrier_Load() const {
-    return reinterpret_cast<void*>(internal::NoBarrier_Load(&rep_));
+    return rep_.load(std::memory_order_relaxed);
   }
   inline void NoBarrier_Store(void* v) {
-    internal::NoBarrier_Store(&rep_, reinterpret_cast<Rep>(v));
+    rep_.store(v, std::memory_order_relaxed);
   }
 };
 
+// We have neither MemoryBarrier(), nor <cstdatomic>
+#else
+#error Please implement AtomicPointer for this platform.
+
+#endif
+
+#undef LEVELDB_HAVE_MEMORY_BARRIER
+#undef ARCH_CPU_X86_FAMILY
+#undef ARCH_CPU_ARM_FAMILY
+
 } // namespace leveldb::port
 } // namespace leveldb
 
-#endif // LEVELDB_CSTDATOMIC_PRESENT
-
 #endif  // PORT_ATOMIC_POINTER_H_

port/port_posix.h

@@ -97,7 +97,7 @@ inline bool Snappy_Uncompress(const char* input_data, size_t input_length,
                               ::std::string* output) {
 #ifdef SNAPPY
   size_t ulength;
-  if (!snappy::GetUncompressedLength(input_data, ulength, &ulength)) {
+  if (!snappy::GetUncompressedLength(input_data, input_length, &ulength)) {
     return false;
   }
   output->resize(ulength);