From bf2c2090b7ee12c5d85b85f08649b6e685f8715f Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 6 Nov 2019 23:23:21 +0100 Subject: [PATCH 01/16] Add filename to corruption errors When a corruption happens, report the filename of the file where corruptions happens. This will aid in diagnosing database corruption issues. Adds a GetName() to all the environment file classes to make it possible to report a filename downstream. --- db/db_impl.cc | 2 +- db/db_test.cc | 3 +++ db/fault_injection_test.cc | 1 + db/leveldbutil.cc | 1 + db/log_reader.cc | 2 +- db/log_test.cc | 2 ++ db/repair.cc | 2 +- helpers/memenv/memenv.cc | 3 +++ include/leveldb/env.h | 9 +++++++++ table/format.cc | 10 +++++----- table/table_test.cc | 2 ++ util/env_posix.cc | 8 ++++++++ util/env_windows.cc | 8 ++++++++ 13 files changed, 45 insertions(+), 8 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 95e2bb410..65e31724b 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -428,7 +428,7 @@ Status DBImpl::RecoverLogFile(uint64_t log_number, bool last_log, while (reader.ReadRecord(&record, &scratch) && status.ok()) { if (record.size() < 12) { reporter.Corruption(record.size(), - Status::Corruption("log record too small")); + Status::Corruption("log record too small", fname)); continue; } WriteBatchInternal::SetContents(&batch, record); diff --git a/db/db_test.cc b/db/db_test.cc index 9a8faf100..beb1d3bde 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -158,6 +158,7 @@ class SpecialEnv : public EnvWrapper { } return base_->Sync(); } + std::string GetName() const override { return ""; } }; class ManifestFile : public WritableFile { private: @@ -183,6 +184,7 @@ class SpecialEnv : public EnvWrapper { return base_->Sync(); } } + std::string GetName() const override { return ""; } }; if (non_writable_.load(std::memory_order_acquire)) { @@ -216,6 +218,7 @@ class SpecialEnv : public EnvWrapper { counter_->Increment(); return target_->Read(offset, n, result, scratch); } + std::string GetName() const override { return ""; } }; Status s = target()->NewRandomAccessFile(f, r); diff --git a/db/fault_injection_test.cc b/db/fault_injection_test.cc index 5b31bb805..bf705cb60 100644 --- a/db/fault_injection_test.cc +++ b/db/fault_injection_test.cc @@ -114,6 +114,7 @@ class TestWritableFile : public WritableFile { Status Close() override; Status Flush() override; Status Sync() override; + std::string GetName() const override { return ""; } private: FileState state_; diff --git a/db/leveldbutil.cc b/db/leveldbutil.cc index 55cdcc577..9ed9667d3 100644 --- a/db/leveldbutil.cc +++ b/db/leveldbutil.cc @@ -20,6 +20,7 @@ class StdoutPrinter : public WritableFile { Status Close() override { return Status::OK(); } Status Flush() override { return Status::OK(); } Status Sync() override { return Status::OK(); } + std::string GetName() const override { return "[stdout]"; } }; bool HandleDumpCommand(Env* env, char** files, int num) { diff --git a/db/log_reader.cc b/db/log_reader.cc index b770fee1a..1ccfb7b34 100644 --- a/db/log_reader.cc +++ b/db/log_reader.cc @@ -176,7 +176,7 @@ bool Reader::ReadRecord(Slice* record, std::string* scratch) { uint64_t Reader::LastRecordOffset() { return last_record_offset_; } void Reader::ReportCorruption(uint64_t bytes, const char* reason) { - ReportDrop(bytes, Status::Corruption(reason)); + ReportDrop(bytes, Status::Corruption(reason, file_->GetName())); } void Reader::ReportDrop(uint64_t bytes, const Status& reason) { diff --git a/db/log_test.cc b/db/log_test.cc index 0e31648cc..41fc04306 100644 --- a/db/log_test.cc +++ b/db/log_test.cc @@ -168,6 +168,7 @@ class LogTest { contents_.append(slice.data(), slice.size()); return Status::OK(); } + std::string GetName() const override { return ""; } std::string contents_; }; @@ -204,6 +205,7 @@ class LogTest { return Status::OK(); } + std::string GetName() const { return ""; } Slice contents_; bool force_error_; diff --git a/db/repair.cc b/db/repair.cc index d9d12ba55..04847c3bb 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -183,7 +183,7 @@ class Repairer { while (reader.ReadRecord(&record, &scratch)) { if (record.size() < 12) { reporter.Corruption(record.size(), - Status::Corruption("log record too small")); + Status::Corruption("log record too small", logname)); continue; } WriteBatchInternal::SetContents(&batch, record); diff --git a/helpers/memenv/memenv.cc b/helpers/memenv/memenv.cc index 31d2bc0f6..47e4481f7 100644 --- a/helpers/memenv/memenv.cc +++ b/helpers/memenv/memenv.cc @@ -178,6 +178,7 @@ class SequentialFileImpl : public SequentialFile { return Status::OK(); } + virtual std::string GetName() const override { return "[memenv]"; } private: FileState* file_; uint64_t pos_; @@ -194,6 +195,7 @@ class RandomAccessFileImpl : public RandomAccessFile { return file_->Read(offset, n, result, scratch); } + virtual std::string GetName() const override { return "[memenv]"; } private: FileState* file_; }; @@ -210,6 +212,7 @@ class WritableFileImpl : public WritableFile { Status Flush() override { return Status::OK(); } Status Sync() override { return Status::OK(); } + virtual std::string GetName() const override { return "[memenv]"; } private: FileState* file_; }; diff --git a/include/leveldb/env.h b/include/leveldb/env.h index 112fe964f..96c21b396 100644 --- a/include/leveldb/env.h +++ b/include/leveldb/env.h @@ -217,6 +217,9 @@ class LEVELDB_EXPORT SequentialFile { // // REQUIRES: External synchronization virtual Status Skip(uint64_t n) = 0; + + // Get a name for the file, only for error reporting + virtual std::string GetName() const = 0; }; // A file abstraction for randomly reading the contents of a file. @@ -240,6 +243,9 @@ class LEVELDB_EXPORT RandomAccessFile { // Safe for concurrent use by multiple threads. virtual Status Read(uint64_t offset, size_t n, Slice* result, char* scratch) const = 0; + + // Get a name for the file, only for error reporting + virtual std::string GetName() const = 0; }; // A file abstraction for sequential writing. The implementation @@ -258,6 +264,9 @@ class LEVELDB_EXPORT WritableFile { virtual Status Close() = 0; virtual Status Flush() = 0; virtual Status Sync() = 0; + + // Get a name for the file, only for error reporting + virtual std::string GetName() const = 0; }; // An interface for writing log messages. diff --git a/table/format.cc b/table/format.cc index e1839779e..a3d67de2e 100644 --- a/table/format.cc +++ b/table/format.cc @@ -79,7 +79,7 @@ Status ReadBlock(RandomAccessFile* file, const ReadOptions& options, } if (contents.size() != n + kBlockTrailerSize) { delete[] buf; - return Status::Corruption("truncated block read"); + return Status::Corruption("truncated block read", file->GetName()); } // Check the crc of the type and the block contents @@ -89,7 +89,7 @@ Status ReadBlock(RandomAccessFile* file, const ReadOptions& options, const uint32_t actual = crc32c::Value(data, n + 1); if (actual != crc) { delete[] buf; - s = Status::Corruption("block checksum mismatch"); + s = Status::Corruption("block checksum mismatch", file->GetName()); return s; } } @@ -116,13 +116,13 @@ Status ReadBlock(RandomAccessFile* file, const ReadOptions& options, size_t ulength = 0; if (!port::Snappy_GetUncompressedLength(data, n, &ulength)) { delete[] buf; - return Status::Corruption("corrupted compressed block contents"); + return Status::Corruption("corrupted compressed block contents", file->GetName()); } char* ubuf = new char[ulength]; if (!port::Snappy_Uncompress(data, n, ubuf)) { delete[] buf; delete[] ubuf; - return Status::Corruption("corrupted compressed block contents"); + return Status::Corruption("corrupted compressed block contents", file->GetName()); } delete[] buf; result->data = Slice(ubuf, ulength); @@ -132,7 +132,7 @@ Status ReadBlock(RandomAccessFile* file, const ReadOptions& options, } default: delete[] buf; - return Status::Corruption("bad block type"); + return Status::Corruption("bad block type", file->GetName()); } return Status::OK(); diff --git a/table/table_test.cc b/table/table_test.cc index f689a2765..17aaea2f9 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -102,6 +102,7 @@ class StringSink : public WritableFile { return Status::OK(); } + std::string GetName() const override { return ""; } private: std::string contents_; }; @@ -128,6 +129,7 @@ class StringSource : public RandomAccessFile { return Status::OK(); } + std::string GetName() const { return ""; } private: std::string contents_; }; diff --git a/util/env_posix.cc b/util/env_posix.cc index 00ca9aede..3651655c6 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -135,6 +135,8 @@ class PosixSequentialFile final : public SequentialFile { return Status::OK(); } + virtual std::string GetName() const override { return filename_; } + private: const int fd_; const std::string filename_; @@ -195,6 +197,8 @@ class PosixRandomAccessFile final : public RandomAccessFile { return status; } + virtual std::string GetName() const override { return filename_; } + private: const bool has_permanent_fd_; // If false, the file is opened on every read. const int fd_; // -1 if has_permanent_fd_ is false. @@ -239,6 +243,8 @@ class PosixMmapReadableFile final : public RandomAccessFile { return Status::OK(); } + virtual std::string GetName() const override { return filename_; } + private: char* const mmap_base_; const size_t length_; @@ -426,6 +432,8 @@ class PosixWritableFile final : public WritableFile { return Basename(filename).starts_with("MANIFEST"); } + virtual std::string GetName() const override { return filename_; } + // buf_[0, pos_ - 1] contains data to be written to fd_. char buf_[kWritableFileBufferSize]; size_t pos_; diff --git a/util/env_windows.cc b/util/env_windows.cc index 2dd7794f6..1f411584f 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -177,6 +177,8 @@ class WindowsSequentialFile : public SequentialFile { return Status::OK(); } + std::string GetName() const override { return filename_; } + private: const ScopedHandle handle_; const std::string filename_; @@ -209,6 +211,8 @@ class WindowsRandomAccessFile : public RandomAccessFile { return Status::OK(); } + std::string GetName() const override { return filename_; } + private: const ScopedHandle handle_; const std::string filename_; @@ -240,6 +244,8 @@ class WindowsMmapReadableFile : public RandomAccessFile { return Status::OK(); } + std::string GetName() const override { return filename_; } + private: char* const mmap_base_; const size_t length_; @@ -309,6 +315,8 @@ class WindowsWritableFile : public WritableFile { return Status::OK(); } + std::string GetName() const override { return filename_; } + private: Status FlushBuffer() { Status status = WriteUnbuffered(buf_, pos_); From d42e63d49d9df05b12cd00af4ffc5f2b3edf7e21 Mon Sep 17 00:00:00 2001 From: Nicolas Dorier Date: Wed, 6 Nov 2019 19:43:32 +0100 Subject: [PATCH 02/16] Do not crash if filesystem can't fsync This code moved, re-apply the fix elsewhere. See - https://github.com/bitcoin/bitcoin/pull/10000 - https://github.com/bitcoin-core/leveldb-old/pull/16 Original change by Nicolas Dorier, ported to leveldb 1.22 by Wladimir J. van der Laan. --- util/env_posix.cc | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index 3651655c6..9938887bf 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -325,7 +325,7 @@ class PosixWritableFile final : public WritableFile { return status; } - return SyncFd(fd_, filename_); + return SyncFd(fd_, filename_, false); } private: @@ -360,7 +360,7 @@ class PosixWritableFile final : public WritableFile { if (fd < 0) { status = PosixError(dirname_, errno); } else { - status = SyncFd(fd, dirname_); + status = SyncFd(fd, dirname_, true); ::close(fd); } return status; @@ -372,7 +372,7 @@ class PosixWritableFile final : public WritableFile { // // The path argument is only used to populate the description string in the // returned Status if an error occurs. - static Status SyncFd(int fd, const std::string& fd_path) { + static Status SyncFd(int fd, const std::string& fd_path, bool syncing_dir) { #if HAVE_FULLFSYNC // On macOS and iOS, fsync() doesn't guarantee durability past power // failures. fcntl(F_FULLFSYNC) is required for that purpose. Some @@ -392,6 +392,11 @@ class PosixWritableFile final : public WritableFile { if (sync_success) { return Status::OK(); } + // Do not crash if filesystem can't fsync directories + // (see https://github.com/bitcoin/bitcoin/pull/10000) + if (syncing_dir && errno == EINVAL) { + return Status::OK(); + } return PosixError(fd_path, errno); } From 92ae82c78f225de84040c51e07fd0b4a61caed99 Mon Sep 17 00:00:00 2001 From: Clem Taylor Date: Wed, 6 Nov 2019 19:47:38 +0100 Subject: [PATCH 03/16] Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems By default LevelDB will only mmap() up to 1000 ldb files for reading and then fall back to using file desciptors. The typical linux system has a 'vm.max_map_count = 65530', so mapping only 1000 files seems arbitarily small. Increase this value to another arbitrarily small value, 4096. See https://github.com/bitcoin-core/leveldb/pull/19. Original change by Clem Taylor. Ported to LevelDB 1.22 by Wladimir J. van der Laan. --- util/env_posix.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index 9938887bf..9f5863a0f 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -42,8 +42,8 @@ namespace { // Set by EnvPosixTestHelper::SetReadOnlyMMapLimit() and MaxOpenFiles(). int g_open_read_only_file_limit = -1; -// Up to 1000 mmap regions for 64-bit binaries; none for 32-bit. -constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 1000 : 0; +// Up to 4096 mmap regions for 64-bit binaries; none for 32-bit. +constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 4096 : 0; // Can be set using EnvPosixTestHelper::SetReadOnlyMMapLimit(). int g_mmap_limit = kDefaultMmapLimit; From f8ae182c1e5176d12e816fb2217ae33a5472fdd7 Mon Sep 17 00:00:00 2001 From: Aaron Clauson Date: Wed, 11 Dec 2019 20:16:20 +0000 Subject: [PATCH 04/16] Adds unicode support to Windows environment. Currently the Windows environment uses the *A ANSI Win32 API calls for file system operations. This precludes the ability of consuming applications to pass paths with unicode characters. A detailed discussion is in #755. This PR swtiches the *A ANSI calls to the *W wide unicode string calls along with the conversion methods from standard UTF-8 strings to the UTF-16 multi-byte strings requires by the Win32 *W API functions. --- util/env_windows.cc | 91 ++++++++++++++++++++++++++++++++------------- 1 file changed, 66 insertions(+), 25 deletions(-) diff --git a/util/env_windows.cc b/util/env_windows.cc index 1f411584f..183420656 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -387,8 +387,9 @@ class WindowsEnv : public Env { *result = nullptr; DWORD desired_access = GENERIC_READ; DWORD share_mode = FILE_SHARE_READ; - ScopedHandle handle = ::CreateFileA( - filename.c_str(), desired_access, share_mode, + auto wFilename = toUtf16(filename); + ScopedHandle handle = ::CreateFileW( + wFilename.c_str(), desired_access, share_mode, /*lpSecurityAttributes=*/nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, /*hTemplateFile=*/nullptr); if (!handle.is_valid()) { @@ -404,8 +405,9 @@ class WindowsEnv : public Env { *result = nullptr; DWORD desired_access = GENERIC_READ; DWORD share_mode = FILE_SHARE_READ; + auto wFilename = toUtf16(filename); ScopedHandle handle = - ::CreateFileA(filename.c_str(), desired_access, share_mode, + ::CreateFileW(wFilename.c_str(), desired_access, share_mode, /*lpSecurityAttributes=*/nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_READONLY, /*hTemplateFile=*/nullptr); @@ -425,7 +427,7 @@ class WindowsEnv : public Env { } ScopedHandle mapping = - ::CreateFileMappingA(handle.get(), + ::CreateFileMappingW(handle.get(), /*security attributes=*/nullptr, PAGE_READONLY, /*dwMaximumSizeHigh=*/0, /*dwMaximumSizeLow=*/0, @@ -450,8 +452,9 @@ class WindowsEnv : public Env { WritableFile** result) override { DWORD desired_access = GENERIC_WRITE; DWORD share_mode = 0; // Exclusive access. - ScopedHandle handle = ::CreateFileA( - filename.c_str(), desired_access, share_mode, + auto wFilename = toUtf16(filename); + ScopedHandle handle = ::CreateFileW( + wFilename.c_str(), desired_access, share_mode, /*lpSecurityAttributes=*/nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, /*hTemplateFile=*/nullptr); if (!handle.is_valid()) { @@ -467,8 +470,9 @@ class WindowsEnv : public Env { WritableFile** result) override { DWORD desired_access = FILE_APPEND_DATA; DWORD share_mode = 0; // Exclusive access. - ScopedHandle handle = ::CreateFileA( - filename.c_str(), desired_access, share_mode, + auto wFilename = toUtf16(filename); + ScopedHandle handle = ::CreateFileW( + wFilename.c_str(), desired_access, share_mode, /*lpSecurityAttributes=*/nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, /*hTemplateFile=*/nullptr); if (!handle.is_valid()) { @@ -481,14 +485,16 @@ class WindowsEnv : public Env { } bool FileExists(const std::string& filename) override { - return GetFileAttributesA(filename.c_str()) != INVALID_FILE_ATTRIBUTES; + auto wFilename = toUtf16(filename); + return GetFileAttributesW(wFilename.c_str()) != INVALID_FILE_ATTRIBUTES; } Status GetChildren(const std::string& directory_path, std::vector* result) override { const std::string find_pattern = directory_path + "\\*"; - WIN32_FIND_DATAA find_data; - HANDLE dir_handle = ::FindFirstFileA(find_pattern.c_str(), &find_data); + WIN32_FIND_DATAW find_data; + auto wFind_pattern = toUtf16(find_pattern); + HANDLE dir_handle = ::FindFirstFileW(wFind_pattern.c_str(), &find_data); if (dir_handle == INVALID_HANDLE_VALUE) { DWORD last_error = ::GetLastError(); if (last_error == ERROR_FILE_NOT_FOUND) { @@ -500,11 +506,12 @@ class WindowsEnv : public Env { char base_name[_MAX_FNAME]; char ext[_MAX_EXT]; - if (!_splitpath_s(find_data.cFileName, nullptr, 0, nullptr, 0, base_name, - ARRAYSIZE(base_name), ext, ARRAYSIZE(ext))) { + auto find_data_filename = toUtf8(find_data.cFileName); + if (!_splitpath_s(find_data_filename.c_str(), nullptr, 0, nullptr, 0, + base_name, ARRAYSIZE(base_name), ext, ARRAYSIZE(ext))) { result->emplace_back(std::string(base_name) + ext); } - } while (::FindNextFileA(dir_handle, &find_data)); + } while (::FindNextFileW(dir_handle, &find_data)); DWORD last_error = ::GetLastError(); ::FindClose(dir_handle); if (last_error != ERROR_NO_MORE_FILES) { @@ -514,21 +521,24 @@ class WindowsEnv : public Env { } Status DeleteFile(const std::string& filename) override { - if (!::DeleteFileA(filename.c_str())) { + auto wFilename = toUtf16(filename); + if (!::DeleteFileW(wFilename.c_str())) { return WindowsError(filename, ::GetLastError()); } return Status::OK(); } Status CreateDir(const std::string& dirname) override { - if (!::CreateDirectoryA(dirname.c_str(), nullptr)) { + auto wDirname = toUtf16(dirname); + if (!::CreateDirectoryW(wDirname.c_str(), nullptr)) { return WindowsError(dirname, ::GetLastError()); } return Status::OK(); } Status DeleteDir(const std::string& dirname) override { - if (!::RemoveDirectoryA(dirname.c_str())) { + auto wDirname = toUtf16(dirname); + if (!::RemoveDirectoryW(wDirname.c_str())) { return WindowsError(dirname, ::GetLastError()); } return Status::OK(); @@ -536,7 +546,8 @@ class WindowsEnv : public Env { Status GetFileSize(const std::string& filename, uint64_t* size) override { WIN32_FILE_ATTRIBUTE_DATA file_attributes; - if (!::GetFileAttributesExA(filename.c_str(), GetFileExInfoStandard, + auto wFilename = toUtf16(filename); + if (!::GetFileAttributesExW(wFilename.c_str(), GetFileExInfoStandard, &file_attributes)) { return WindowsError(filename, ::GetLastError()); } @@ -550,7 +561,9 @@ class WindowsEnv : public Env { Status RenameFile(const std::string& from, const std::string& to) override { // Try a simple move first. It will only succeed when |to| doesn't already // exist. - if (::MoveFileA(from.c_str(), to.c_str())) { + auto wFrom = toUtf16(from); + auto wTo = toUtf16(to); + if (::MoveFileW(wFrom.c_str(), wTo.c_str())) { return Status::OK(); } DWORD move_error = ::GetLastError(); @@ -559,7 +572,7 @@ class WindowsEnv : public Env { // succeed when |to| does exist. When writing to a network share, we may not // be able to change the ACLs. Ignore ACL errors then // (REPLACEFILE_IGNORE_MERGE_ERRORS). - if (::ReplaceFileA(to.c_str(), from.c_str(), /*lpBackupFileName=*/nullptr, + if (::ReplaceFileW(wTo.c_str(), wFrom.c_str(), /*lpBackupFileName=*/nullptr, REPLACEFILE_IGNORE_MERGE_ERRORS, /*lpExclude=*/nullptr, /*lpReserved=*/nullptr)) { return Status::OK(); @@ -579,8 +592,9 @@ class WindowsEnv : public Env { Status LockFile(const std::string& filename, FileLock** lock) override { *lock = nullptr; Status result; - ScopedHandle handle = ::CreateFileA( - filename.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, + auto wFilename = toUtf16(filename); + ScopedHandle handle = ::CreateFileW( + wFilename.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, /*lpSecurityAttributes=*/nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); if (!handle.is_valid()) { @@ -620,10 +634,11 @@ class WindowsEnv : public Env { return Status::OK(); } - char tmp_path[MAX_PATH]; - if (!GetTempPathA(ARRAYSIZE(tmp_path), tmp_path)) { + wchar_t wtmp_path[MAX_PATH]; + if (!GetTempPathW(ARRAYSIZE(wtmp_path), wtmp_path)) { return WindowsError("GetTempPath", ::GetLastError()); } + std::string tmp_path = toUtf8(std::wstring(wtmp_path)); std::stringstream ss; ss << tmp_path << "leveldbtest-" << std::this_thread::get_id(); *result = ss.str(); @@ -634,7 +649,8 @@ class WindowsEnv : public Env { } Status NewLogger(const std::string& filename, Logger** result) override { - std::FILE* fp = std::fopen(filename.c_str(), "w"); + auto wFilename = toUtf16(filename); + std::FILE* fp = _wfopen(wFilename.c_str(), L"w"); if (fp == nullptr) { *result = nullptr; return WindowsError(filename, ::GetLastError()); @@ -690,6 +706,31 @@ class WindowsEnv : public Env { GUARDED_BY(background_work_mutex_); Limiter mmap_limiter_; // Thread-safe. + + // Converts a Windows wide multi-byte UTF-16 string to a UTF-8 string. + // See http://utf8everywhere.org/#windows + std::string toUtf8(const std::wstring& wstr) { + if (wstr.empty()) return std::string(); + int size_needed = WideCharToMultiByte( + CP_UTF8, 0, &wstr[0], (int)wstr.size(), NULL, 0, NULL, NULL); + std::string strTo(size_needed, 0); + WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), &strTo[0], + size_needed, NULL, NULL); + return strTo; + } + + // Converts a UTF-8 string to a Windows UTF-16 multi-byte wide character + // string. + // See http://utf8everywhere.org/#windows + std::wstring toUtf16(const std::string& str) { + if (str.empty()) return std::wstring(); + int size_needed = + MultiByteToWideChar(CP_UTF8, 0, &str[0], (int)str.size(), NULL, 0); + std::wstring strTo(size_needed, 0); + MultiByteToWideChar(CP_UTF8, 0, &str[0], (int)str.size(), &strTo[0], + size_needed); + return strTo; + } }; // Return the maximum number of concurrent mmaps. From 93ee8d875e9994dc8353af5a680f9950de322194 Mon Sep 17 00:00:00 2001 From: Victor Costan Date: Sat, 8 Jan 2022 20:55:42 +0000 Subject: [PATCH 05/16] Merge pull request #965 from ShawnZhong:cpp20 PiperOrigin-RevId: 420504266 (cherry picked from commit e4ccaa0c9cb1a019df3df74eb5db4ec55e9e02ed) --- util/env_posix.cc | 4 ++-- util/env_windows.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index 00ca9aede..c57f4594d 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -837,7 +837,7 @@ class SingletonEnv { public: SingletonEnv() { #if !defined(NDEBUG) - env_initialized_.store(true, std::memory_order::memory_order_relaxed); + env_initialized_.store(true, std::memory_order_relaxed); #endif // !defined(NDEBUG) static_assert(sizeof(env_storage_) >= sizeof(EnvType), "env_storage_ will not fit the Env"); @@ -854,7 +854,7 @@ class SingletonEnv { static void AssertEnvNotInitialized() { #if !defined(NDEBUG) - assert(!env_initialized_.load(std::memory_order::memory_order_relaxed)); + assert(!env_initialized_.load(std::memory_order_relaxed)); #endif // !defined(NDEBUG) } diff --git a/util/env_windows.cc b/util/env_windows.cc index 2dd7794f6..f0f14ff4e 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -749,7 +749,7 @@ class SingletonEnv { public: SingletonEnv() { #if !defined(NDEBUG) - env_initialized_.store(true, std::memory_order::memory_order_relaxed); + env_initialized_.store(true, std::memory_order_relaxed); #endif // !defined(NDEBUG) static_assert(sizeof(env_storage_) >= sizeof(EnvType), "env_storage_ will not fit the Env"); @@ -766,7 +766,7 @@ class SingletonEnv { static void AssertEnvNotInitialized() { #if !defined(NDEBUG) - assert(!env_initialized_.load(std::memory_order::memory_order_relaxed)); + assert(!env_initialized_.load(std::memory_order_relaxed)); #endif // !defined(NDEBUG) } From 1eeb1cb879675e68469fc19560047f98ccc3a2c3 Mon Sep 17 00:00:00 2001 From: "emptyx.wong" Date: Thu, 9 Jun 2022 14:50:01 +0800 Subject: [PATCH 06/16] fix macro HAVE_O_CLOEXEC when O_CLOEXEC not found --- util/env_posix.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index 18626b327..fac41be6c 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -49,7 +49,7 @@ constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 4096 : 0; int g_mmap_limit = kDefaultMmapLimit; // Common flags defined for all posix open operations -#if defined(HAVE_O_CLOEXEC) +#if HAVE_O_CLOEXEC constexpr const int kOpenBaseFlags = O_CLOEXEC; #else constexpr const int kOpenBaseFlags = 0; From 12c52b392d5c7b4618a63ad22f0fe21c71b0913b Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 15 Sep 2022 10:19:46 +0100 Subject: [PATCH 07/16] win32: fix -Wmissing-field-initializers warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building with `-Wmissing-field-initializers`, the following warning is emitted: ```bash leveldb/util/env_windows.cc: In member function ‘virtual leveldb::Status leveldb::{anonymous}::WindowsRandomAccessFile::Read(uint64_t, size_t, leveldb::Slice*, char*) const’: leveldb/util/env_windows.cc:197:31: warning: missing initializer for member ‘_OVERLAPPED::InternalHigh’ [-Wmissing-field-initializers] 197 | OVERLAPPED overlapped = {0}; | ^ leveldb/util/env_windows.cc:197:31: warning: missing initializer for member ‘_OVERLAPPED::’ [-Wmissing-field-initializers] leveldb/util/env_windows.cc:197:31: warning: missing initializer for member ‘_OVERLAPPED::hEvent’ [-Wmissing-field-initializers] ``` Submitted upstream: https://github.com/google/leveldb/pull/1053 --- util/env_windows.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/env_windows.cc b/util/env_windows.cc index 4dcba222a..aafcdcc3b 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -194,7 +194,7 @@ class WindowsRandomAccessFile : public RandomAccessFile { Status Read(uint64_t offset, size_t n, Slice* result, char* scratch) const override { DWORD bytes_read = 0; - OVERLAPPED overlapped = {0}; + OVERLAPPED overlapped = {}; overlapped.OffsetHigh = static_cast(offset >> 32); overlapped.Offset = static_cast(offset); From 2e3c0131d3331b048ed4dc13f4c26691e7c958ba Mon Sep 17 00:00:00 2001 From: Victor Costan Date: Mon, 13 Apr 2020 23:18:12 +0000 Subject: [PATCH 08/16] Remove leveldb::port::kLittleEndian. Clang 10 includes the optimizations described in https://bugs.llvm.org/show_bug.cgi?id=41761. This means that the platform-independent implementations of {Decode,Encode}Fixed{32,64}() compile to one instruction on the most recent Clang and GCC. PiperOrigin-RevId: 306330166 --- CMakeLists.txt | 3 --- port/port_config.h.in | 6 ------ port/port_example.h | 4 ---- port/port_stdcxx.h | 2 -- util/coding.h | 50 ++++--------------------------------------- 5 files changed, 4 insertions(+), 61 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1cb46256c..cfd4faa32 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,9 +28,6 @@ option(LEVELDB_BUILD_TESTS "Build LevelDB's unit tests" ON) option(LEVELDB_BUILD_BENCHMARKS "Build LevelDB's benchmarks" ON) option(LEVELDB_INSTALL "Install LevelDB's header and library" ON) -include(TestBigEndian) -test_big_endian(LEVELDB_IS_BIG_ENDIAN) - include(CheckIncludeFile) check_include_file("unistd.h" HAVE_UNISTD_H) diff --git a/port/port_config.h.in b/port/port_config.h.in index 21273153a..272671d39 100644 --- a/port/port_config.h.in +++ b/port/port_config.h.in @@ -30,10 +30,4 @@ #cmakedefine01 HAVE_SNAPPY #endif // !defined(HAVE_SNAPPY) -// Define to 1 if your processor stores words with the most significant byte -// first (like Motorola and SPARC, unlike Intel and VAX). -#if !defined(LEVELDB_IS_BIG_ENDIAN) -#cmakedefine01 LEVELDB_IS_BIG_ENDIAN -#endif // !defined(LEVELDB_IS_BIG_ENDIAN) - #endif // STORAGE_LEVELDB_PORT_PORT_CONFIG_H_ \ No newline at end of file diff --git a/port/port_example.h b/port/port_example.h index 1a8fca24b..a665910d9 100644 --- a/port/port_example.h +++ b/port/port_example.h @@ -18,10 +18,6 @@ namespace port { // TODO(jorlow): Many of these belong more in the environment class rather than // here. We should try moving them and see if it affects perf. -// The following boolean constant must be true on a little-endian machine -// and false otherwise. -static const bool kLittleEndian = true /* or some other expression */; - // ------------------ Threading ------------------- // A Mutex represents an exclusive lock. diff --git a/port/port_stdcxx.h b/port/port_stdcxx.h index e9cb0e53a..2bda48db4 100644 --- a/port/port_stdcxx.h +++ b/port/port_stdcxx.h @@ -41,8 +41,6 @@ namespace leveldb { namespace port { -static const bool kLittleEndian = !LEVELDB_IS_BIG_ENDIAN; - class CondVar; // Thinly wraps std::mutex. diff --git a/util/coding.h b/util/coding.h index 1983ae717..f0bb57b8e 100644 --- a/util/coding.h +++ b/util/coding.h @@ -48,29 +48,13 @@ int VarintLength(uint64_t v); char* EncodeVarint32(char* dst, uint32_t value); char* EncodeVarint64(char* dst, uint64_t value); -// TODO(costan): Remove port::kLittleEndian and the fast paths based on -// std::memcpy when clang learns to optimize the generic code, as -// described in https://bugs.llvm.org/show_bug.cgi?id=41761 -// -// The platform-independent code in DecodeFixed{32,64}() gets optimized to mov -// on x86 and ldr on ARM64, by both clang and gcc. However, only gcc optimizes -// the platform-independent code in EncodeFixed{32,64}() to mov / str. - // Lower-level versions of Put... that write directly into a character buffer // REQUIRES: dst has enough space for the value being written inline void EncodeFixed32(char* dst, uint32_t value) { uint8_t* const buffer = reinterpret_cast(dst); - if (port::kLittleEndian) { - // Fast path for little-endian CPUs. All major compilers optimize this to a - // single mov (x86_64) / str (ARM) instruction. - std::memcpy(buffer, &value, sizeof(uint32_t)); - return; - } - - // Platform-independent code. - // Currently, only gcc optimizes this to a single mov / str instruction. + // Recent clang and gcc optimize this to a single mov / str instruction. buffer[0] = static_cast(value); buffer[1] = static_cast(value >> 8); buffer[2] = static_cast(value >> 16); @@ -80,15 +64,7 @@ inline void EncodeFixed32(char* dst, uint32_t value) { inline void EncodeFixed64(char* dst, uint64_t value) { uint8_t* const buffer = reinterpret_cast(dst); - if (port::kLittleEndian) { - // Fast path for little-endian CPUs. All major compilers optimize this to a - // single mov (x86_64) / str (ARM) instruction. - std::memcpy(buffer, &value, sizeof(uint64_t)); - return; - } - - // Platform-independent code. - // Currently, only gcc optimizes this to a single mov / str instruction. + // Recent clang and gcc optimize this to a single mov / str instruction. buffer[0] = static_cast(value); buffer[1] = static_cast(value >> 8); buffer[2] = static_cast(value >> 16); @@ -105,16 +81,7 @@ inline void EncodeFixed64(char* dst, uint64_t value) { inline uint32_t DecodeFixed32(const char* ptr) { const uint8_t* const buffer = reinterpret_cast(ptr); - if (port::kLittleEndian) { - // Fast path for little-endian CPUs. All major compilers optimize this to a - // single mov (x86_64) / ldr (ARM) instruction. - uint32_t result; - std::memcpy(&result, buffer, sizeof(uint32_t)); - return result; - } - - // Platform-independent code. - // Clang and gcc optimize this to a single mov / ldr instruction. + // Recent clang and gcc optimize this to a single mov / ldr instruction. return (static_cast(buffer[0])) | (static_cast(buffer[1]) << 8) | (static_cast(buffer[2]) << 16) | @@ -124,16 +91,7 @@ inline uint32_t DecodeFixed32(const char* ptr) { inline uint64_t DecodeFixed64(const char* ptr) { const uint8_t* const buffer = reinterpret_cast(ptr); - if (port::kLittleEndian) { - // Fast path for little-endian CPUs. All major compilers optimize this to a - // single mov (x86_64) / ldr (ARM) instruction. - uint64_t result; - std::memcpy(&result, buffer, sizeof(uint64_t)); - return result; - } - - // Platform-independent code. - // Clang and gcc optimize this to a single mov / ldr instruction. + // Recent clang and gcc optimize this to a single mov / ldr instruction. return (static_cast(buffer[0])) | (static_cast(buffer[1]) << 8) | (static_cast(buffer[2]) << 16) | From 7045a90ed784bef1bad3ec870030ed5c56ea267a Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 11 Jun 2024 14:03:42 +0000 Subject: [PATCH 09/16] Ignore clang's self-assignment check As-documented in the code, this is already safe so ignore the false-positive. --- include/leveldb/status.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/leveldb/status.h b/include/leveldb/status.h index e3273144e..68efe3001 100644 --- a/include/leveldb/status.h +++ b/include/leveldb/status.h @@ -103,6 +103,8 @@ class LEVELDB_EXPORT Status { inline Status::Status(const Status& rhs) { state_ = (rhs.state_ == nullptr) ? nullptr : CopyState(rhs.state_); } + +// NOLINTBEGIN(bugprone-unhandled-self-assignment) inline Status& Status::operator=(const Status& rhs) { // The following condition catches both aliasing (when this == &rhs), // and the common case where both rhs and *this are ok. @@ -112,6 +114,8 @@ inline Status& Status::operator=(const Status& rhs) { } return *this; } +// NOLINTEND(bugprone-unhandled-self-assignment) + inline Status& Status::operator=(Status&& rhs) noexcept { std::swap(state_, rhs.state_); return *this; From be4dfc94b3a4355e15c09023ee41140e5e07e6f4 Mon Sep 17 00:00:00 2001 From: Peter Kasting Date: Thu, 25 Jul 2024 09:39:54 -0700 Subject: [PATCH 10/16] [jumbo] Add begin()/end() to Slice. This allows this type to meet the requirements of e.g. std::ranges::range, which is necessary for it to work with the std::span range constructor, or the "non-legacy" constructor for Chromium's base::span. Bug: none (cherry picked from commit 2cc36eb56668306c64fc611fb7ad63ecf0b20379) --- include/leveldb/slice.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/leveldb/slice.h b/include/leveldb/slice.h index 2df417dc3..44de9038c 100644 --- a/include/leveldb/slice.h +++ b/include/leveldb/slice.h @@ -52,6 +52,9 @@ class LEVELDB_EXPORT Slice { // Return true iff the length of the referenced data is zero bool empty() const { return size_ == 0; } + const char* begin() const { return data(); } + const char* end() const { return data() + size(); } + // Return the ith byte in the referenced data. // REQUIRES: n < size() char operator[](size_t n) const { From a8844b23ab02a48a05940987f70bf23d4fa76f58 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 2 Jan 2025 14:02:29 -0500 Subject: [PATCH 11/16] Fix invalid pointer arithmetic in Hash (#1222) It is UB to exceed the bounds of the buffer when doing pointer arithemetic. That means the following is not a valid bounds check: if (start + 4 <= limit) Because if we were at the end of the buffer, we wouldn't be allowed to add 4 anyway. Instead, this must be written as: if (limit - start >= 4) Basic forms of this issue are flagged by UBSan. If building with -fsanitize=undefined, the following test trips an error: [ RUN ] HASH.SignedUnsignedIssue .../leveldb/util/hash.cc:30:15: runtime error: applying non-zero offset 4 to null pointer SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/local/google/home/davidben/leveldb/util/hash.cc:30:15 in [ OK ] HASH.SignedUnsignedIssue (1 ms) (cherry picked from commit 578eeb702ec0fbb6b9780f3d4147b1076630d633) --- util/hash.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/hash.cc b/util/hash.cc index dd47c110e..5432b6180 100644 --- a/util/hash.cc +++ b/util/hash.cc @@ -27,7 +27,7 @@ uint32_t Hash(const char* data, size_t n, uint32_t seed) { uint32_t h = seed ^ (n * m); // Pick up four bytes at a time - while (data + 4 <= limit) { + while (limit - data >= 4) { uint32_t w = DecodeFixed32(data); data += 4; h += w; From 82c31046edfa7548702c16cfc051f19f985566ca Mon Sep 17 00:00:00 2001 From: leveldb Team Date: Wed, 8 Jan 2025 18:54:39 +0000 Subject: [PATCH 12/16] Fix C++23 compilation errors in leveldb Remove usages of std::aligned_storage, which is deprecated. More details about the replacement in https://crbug.com/388068052. PiperOrigin-RevId: 713346733 --- util/env_posix.cc | 9 ++++++--- util/env_windows.cc | 9 ++++++--- util/no_destructor.h | 10 +++++++--- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index fac41be6c..ca70ff67f 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -854,7 +854,11 @@ class SingletonEnv { #endif // !defined(NDEBUG) static_assert(sizeof(env_storage_) >= sizeof(EnvType), "env_storage_ will not fit the Env"); - static_assert(alignof(decltype(env_storage_)) >= alignof(EnvType), + static_assert(std::is_standard_layout_v>); + static_assert( + offsetof(SingletonEnv, env_storage_) % alignof(EnvType) == 0, + "env_storage_ does not meet the Env's alignment needs"); + static_assert(alignof(SingletonEnv) % alignof(EnvType) == 0, "env_storage_ does not meet the Env's alignment needs"); new (&env_storage_) EnvType(); } @@ -872,8 +876,7 @@ class SingletonEnv { } private: - typename std::aligned_storage::type - env_storage_; + alignas(EnvType) char env_storage_[sizeof(EnvType)]; #if !defined(NDEBUG) static std::atomic env_initialized_; #endif // !defined(NDEBUG) diff --git a/util/env_windows.cc b/util/env_windows.cc index aafcdcc3b..2137a9f15 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -802,7 +802,11 @@ class SingletonEnv { #endif // !defined(NDEBUG) static_assert(sizeof(env_storage_) >= sizeof(EnvType), "env_storage_ will not fit the Env"); - static_assert(alignof(decltype(env_storage_)) >= alignof(EnvType), + static_assert(std::is_standard_layout_v>); + static_assert( + offsetof(SingletonEnv, env_storage_) % alignof(EnvType) == 0, + "env_storage_ does not meet the Env's alignment needs"); + static_assert(alignof(SingletonEnv) % alignof(EnvType) == 0, "env_storage_ does not meet the Env's alignment needs"); new (&env_storage_) EnvType(); } @@ -820,8 +824,7 @@ class SingletonEnv { } private: - typename std::aligned_storage::type - env_storage_; + alignas(EnvType) char env_storage_[sizeof(EnvType)]; #if !defined(NDEBUG) static std::atomic env_initialized_; #endif // !defined(NDEBUG) diff --git a/util/no_destructor.h b/util/no_destructor.h index a0d3b8703..08ce6a40a 100644 --- a/util/no_destructor.h +++ b/util/no_destructor.h @@ -5,6 +5,7 @@ #ifndef STORAGE_LEVELDB_UTIL_NO_DESTRUCTOR_H_ #define STORAGE_LEVELDB_UTIL_NO_DESTRUCTOR_H_ +#include #include #include @@ -20,8 +21,12 @@ class NoDestructor { explicit NoDestructor(ConstructorArgTypes&&... constructor_args) { static_assert(sizeof(instance_storage_) >= sizeof(InstanceType), "instance_storage_ is not large enough to hold the instance"); + static_assert(std::is_standard_layout_v>); static_assert( - alignof(decltype(instance_storage_)) >= alignof(InstanceType), + offsetof(NoDestructor, instance_storage_) % alignof(InstanceType) == 0, + "instance_storage_ does not meet the instance's alignment requirement"); + static_assert( + alignof(NoDestructor) % alignof(InstanceType) == 0, "instance_storage_ does not meet the instance's alignment requirement"); new (&instance_storage_) InstanceType(std::forward(constructor_args)...); @@ -37,8 +42,7 @@ class NoDestructor { } private: - typename std::aligned_storage::type instance_storage_; + alignas(InstanceType) char instance_storage_[sizeof(InstanceType)]; }; } // namespace leveldb From 183e79a495d4e19c539cf1d64498106972c0ee6b Mon Sep 17 00:00:00 2001 From: leveldb Team Date: Tue, 21 Jan 2025 16:19:03 +0000 Subject: [PATCH 13/16] Fix speculatively some "placement new" issues in leveldb cl/713346733 changed the type of some variables to pointers, but didn't adjust the placement new statements. From pkasting@: "I suspect your code is wrong and will crash. An array is a pointer, so taking its address also gives a pointer, which is why it compiles; but the value of that pointer is different. You're no longer providing the address of the storage, but rather the address of the array pointer." PiperOrigin-RevId: 717926210 --- util/env_posix.cc | 2 +- util/env_windows.cc | 2 +- util/no_destructor.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index ca70ff67f..86571059b 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -860,7 +860,7 @@ class SingletonEnv { "env_storage_ does not meet the Env's alignment needs"); static_assert(alignof(SingletonEnv) % alignof(EnvType) == 0, "env_storage_ does not meet the Env's alignment needs"); - new (&env_storage_) EnvType(); + new (env_storage_) EnvType(); } ~SingletonEnv() = default; diff --git a/util/env_windows.cc b/util/env_windows.cc index 2137a9f15..0a48c3fb5 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -808,7 +808,7 @@ class SingletonEnv { "env_storage_ does not meet the Env's alignment needs"); static_assert(alignof(SingletonEnv) % alignof(EnvType) == 0, "env_storage_ does not meet the Env's alignment needs"); - new (&env_storage_) EnvType(); + new (env_storage_) EnvType(); } ~SingletonEnv() = default; diff --git a/util/no_destructor.h b/util/no_destructor.h index 08ce6a40a..c28a10731 100644 --- a/util/no_destructor.h +++ b/util/no_destructor.h @@ -28,7 +28,7 @@ class NoDestructor { static_assert( alignof(NoDestructor) % alignof(InstanceType) == 0, "instance_storage_ does not meet the instance's alignment requirement"); - new (&instance_storage_) + new (instance_storage_) InstanceType(std::forward(constructor_args)...); } From 9defe8494abd5abd40eb4da866555bc8b30f4807 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 2 May 2025 13:15:40 +0100 Subject: [PATCH 14/16] refactor: add missing overrides Fixes instances of: ```bash leveldb-subtree/db/log_test.cc:208:17: error: 'GetName' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override] 208 | std::string GetName() const { return ""; } ``` --- db/c.cc | 6 +++--- db/db_test.cc | 16 ++++++++-------- db/log_test.cc | 2 +- table/table_test.cc | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/db/c.cc b/db/c.cc index 3a492f9ac..8def42755 100644 --- a/db/c.cc +++ b/db/c.cc @@ -471,11 +471,11 @@ leveldb_filterpolicy_t* leveldb_filterpolicy_create_bloom(int bits_per_key) { static void DoNothing(void*) {} ~Wrapper() { delete rep_; } - const char* Name() const { return rep_->Name(); } - void CreateFilter(const Slice* keys, int n, std::string* dst) const { + const char* Name() const override { return rep_->Name(); } + void CreateFilter(const Slice* keys, int n, std::string* dst) const override { return rep_->CreateFilter(keys, n, dst); } - bool KeyMayMatch(const Slice& key, const Slice& filter) const { + bool KeyMayMatch(const Slice& key, const Slice& filter) const override { return rep_->KeyMayMatch(key, filter); } diff --git a/db/db_test.cc b/db/db_test.cc index beb1d3bde..5ed9c1d75 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -139,7 +139,7 @@ class SpecialEnv : public EnvWrapper { public: DataFile(SpecialEnv* env, WritableFile* base) : env_(env), base_(base) {} ~DataFile() { delete base_; } - Status Append(const Slice& data) { + Status Append(const Slice& data) override { if (env_->no_space_.load(std::memory_order_acquire)) { // Drop writes on the floor return Status::OK(); @@ -147,9 +147,9 @@ class SpecialEnv : public EnvWrapper { return base_->Append(data); } } - Status Close() { return base_->Close(); } - Status Flush() { return base_->Flush(); } - Status Sync() { + Status Close() override { return base_->Close(); } + Status Flush() override { return base_->Flush(); } + Status Sync() override { if (env_->data_sync_error_.load(std::memory_order_acquire)) { return Status::IOError("simulated data sync error"); } @@ -168,16 +168,16 @@ class SpecialEnv : public EnvWrapper { public: ManifestFile(SpecialEnv* env, WritableFile* b) : env_(env), base_(b) {} ~ManifestFile() { delete base_; } - Status Append(const Slice& data) { + Status Append(const Slice& data) override { if (env_->manifest_write_error_.load(std::memory_order_acquire)) { return Status::IOError("simulated writer error"); } else { return base_->Append(data); } } - Status Close() { return base_->Close(); } - Status Flush() { return base_->Flush(); } - Status Sync() { + Status Close() override { return base_->Close(); } + Status Flush() override { return base_->Flush(); } + Status Sync() override { if (env_->manifest_sync_error_.load(std::memory_order_acquire)) { return Status::IOError("simulated sync error"); } else { diff --git a/db/log_test.cc b/db/log_test.cc index 41fc04306..45b6bb197 100644 --- a/db/log_test.cc +++ b/db/log_test.cc @@ -205,7 +205,7 @@ class LogTest { return Status::OK(); } - std::string GetName() const { return ""; } + std::string GetName() const override { return ""; } Slice contents_; bool force_error_; diff --git a/table/table_test.cc b/table/table_test.cc index 17aaea2f9..3156c0a6b 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -129,7 +129,7 @@ class StringSource : public RandomAccessFile { return Status::OK(); } - std::string GetName() const { return ""; } + std::string GetName() const override { return ""; } private: std::string contents_; }; From 7daf4ed57517fae379335fb94ca12c6dcbf26637 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 28 May 2025 20:15:43 +0000 Subject: [PATCH 15/16] Fix clang thread-safety-pointer warnings Clang's -Wthread-safety-pointer warnings will become part of -Wthread-safety in some future release. Add missing annotations for functions that pass the address of guarded members. No functional change. Fixes the following warnings: util/cache.cc:220:17: warning: passing pointer to variable 'in_use_' requires holding mutex 'mutex_' [-Wthread-safety-pointer] 220 | LRU_Append(&in_use_, e); | ^ util/cache.cc:235:17: warning: passing pointer to variable 'lru_' requires holding mutex 'mutex_' [-Wthread-safety-pointer] 235 | LRU_Append(&lru_, e); --- util/cache.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/cache.cc b/util/cache.cc index 12de306ca..2d4433fcc 100644 --- a/util/cache.cc +++ b/util/cache.cc @@ -171,8 +171,8 @@ class LRUCache { private: void LRU_Remove(LRUHandle* e); void LRU_Append(LRUHandle* list, LRUHandle* e); - void Ref(LRUHandle* e); - void Unref(LRUHandle* e); + void Ref(LRUHandle* e) EXCLUSIVE_LOCKS_REQUIRED(mutex_); + void Unref(LRUHandle* e) EXCLUSIVE_LOCKS_REQUIRED(mutex_); bool FinishErase(LRUHandle* e) EXCLUSIVE_LOCKS_REQUIRED(mutex_); // Initialized before use. From 157ed16be9e5e54576307a6a5dc2e83efb3f165d Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 7 Aug 2025 15:26:43 +0100 Subject: [PATCH 16/16] doc: fix typos Fix everything that keeps getting sent downstream. i.e https://github.com/bitcoin/bitcoin/pull/33638 https://github.com/bitcoin/bitcoin/pull/33148 https://github.com/bitcoin/bitcoin/pull/22664 https://github.com/bitcoin/bitcoin/pull/13781 Co-authored-by: hoffman Co-authored-by: vastonus --- README.md | 2 +- db/db_test.cc | 2 +- db/snapshot.h | 2 +- doc/benchmark.html | 2 +- doc/index.md | 2 +- util/env_posix.cc | 4 ++-- util/env_windows.cc | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index dadfd5693..f423437d4 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ mkdir build cd build cmake -G "Visual Studio 15" .. ``` -The default default will build for x86. For 64-bit run: +The default will build for x86. For 64-bit run: ```cmd cmake -G "Visual Studio 15 Win64" .. diff --git a/db/db_test.cc b/db/db_test.cc index 5ed9c1d75..3c9f89428 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -2194,7 +2194,7 @@ TEST(DBTest, Randomized) { if (i == 0 || !rnd.OneIn(10)) { k = RandomKey(&rnd); } else { - // Periodically re-use the same key from the previous iter, so + // Periodically reuse the same key from the previous iter, so // we have multiple entries in the write batch for the same key } if (rnd.OneIn(2)) { diff --git a/db/snapshot.h b/db/snapshot.h index 9f1d66491..817bb7ba1 100644 --- a/db/snapshot.h +++ b/db/snapshot.h @@ -25,7 +25,7 @@ class SnapshotImpl : public Snapshot { friend class SnapshotList; // SnapshotImpl is kept in a doubly-linked circular list. The SnapshotList - // implementation operates on the next/previous fields direcly. + // implementation operates on the next/previous fields directly. SnapshotImpl* prev_; SnapshotImpl* next_; diff --git a/doc/benchmark.html b/doc/benchmark.html index f3fd77144..6557d6ef9 100644 --- a/doc/benchmark.html +++ b/doc/benchmark.html @@ -83,7 +83,7 @@

LevelDB Benchmarks

Google, July 2011


-

In order to test LevelDB's performance, we benchmark it against other well-established database implementations. We compare LevelDB (revision 39) against SQLite3 (version 3.7.6.3) and Kyoto Cabinet's (version 1.2.67) TreeDB (a B+Tree based key-value store). We would like to acknowledge Scott Hess and Mikio Hirabayashi for their suggestions and contributions to the SQLite3 and Kyoto Cabinet benchmarks, respectively.

+

In order to test LevelDB's performance, we benchmark it against other well-established database implementations. We compare LevelDB (revision 39) against SQLite3 (version 3.7.6.3) and Kyoto Cabinet's (version 1.2.67) TreeDB (a B+Tree based key-value store). We would like to acknowledge Scott Hess and Mikio Hirabayashi for their suggestions and contributions to the SQLite3 and Kyoto Cabinet benchmarks, respectively.

Benchmarks were all performed on a six-core Intel(R) Xeon(R) CPU X5650 @ 2.67GHz, with 12288 KB of total L3 cache and 12 GB of DDR3 RAM at 1333 MHz. (Note that LevelDB uses at most two CPUs since the benchmarks are single threaded: one to run the benchmark, and one for background compactions.) We ran the benchmarks on two machines (with identical processors), one with an Ext3 file system and one with an Ext4 file system. The machine with the Ext3 file system has a SATA Hitachi HDS721050CLA362 hard drive. The machine with the Ext4 file system has a SATA Samsung HD502HJ hard drive. Both hard drives spin at 7200 RPM and have hard drive write-caching enabled (using `hdparm -W 1 [device]`). The numbers reported below are the median of three measurements.

diff --git a/doc/index.md b/doc/index.md index 3d9a25805..e728178a8 100644 --- a/doc/index.md +++ b/doc/index.md @@ -325,7 +325,7 @@ sizes. Each block is individually compressed before being written to persistent storage. Compression is on by default since the default compression method is -very fast, and is automatically disabled for uncompressible data. In rare cases, +very fast, and is automatically disabled for incompressible data. In rare cases, applications may want to disable compression entirely, but should only do so if benchmarks show a performance improvement: diff --git a/util/env_posix.cc b/util/env_posix.cc index 86571059b..8a33534ed 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -218,7 +218,7 @@ class PosixMmapReadableFile final : public RandomAccessFile { // over the ownership of the region. // // |mmap_limiter| must outlive this instance. The caller must have already - // aquired the right to use one mmap region, which will be released when this + // acquired the right to use one mmap region, which will be released when this // instance is destroyed. PosixMmapReadableFile(std::string filename, char* mmap_base, size_t length, Limiter* mmap_limiter) @@ -741,7 +741,7 @@ class PosixEnv : public Env { // Instances are constructed on the thread calling Schedule() and used on the // background thread. // - // This structure is thread-safe beacuse it is immutable. + // This structure is thread-safe because it is immutable. struct BackgroundWorkItem { explicit BackgroundWorkItem(void (*function)(void* arg), void* arg) : function(function), arg(arg) {} diff --git a/util/env_windows.cc b/util/env_windows.cc index 0a48c3fb5..8897e2aa2 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -689,7 +689,7 @@ class WindowsEnv : public Env { // Instances are constructed on the thread calling Schedule() and used on the // background thread. // - // This structure is thread-safe beacuse it is immutable. + // This structure is thread-safe because it is immutable. struct BackgroundWorkItem { explicit BackgroundWorkItem(void (*function)(void* arg), void* arg) : function(function), arg(arg) {}