aboutsummaryrefslogtreecommitdiff
path: root/src/zenstore/compactcas.cpp
diff options
context:
space:
mode:
authorStefan Boberg <[email protected]>2026-03-06 12:39:06 +0100
committerGitHub Enterprise <[email protected]>2026-03-06 12:39:06 +0100
commit5115b419cefd41e8d5cc465c8c7ae5140cde71d4 (patch)
treec97f8d658fa0ec24664264b97327120f2c30bd7f /src/zenstore/compactcas.cpp
parentClaude config, some bug fixes (#813) (diff)
downloadzen-5115b419cefd41e8d5cc465c8c7ae5140cde71d4.tar.xz
zen-5115b419cefd41e8d5cc465c8c7ae5140cde71d4.zip
zenstore bug-fixes from static analysis pass (#815)
**Bug fixes across zenstore, zenremotestore, and related subsystems, primarily surfaced by static analysis.** ## Cache subsystem (cachedisklayer.cpp) - Fixed tombstone scoping bug: tombstone flag and missing entry were recorded outside the block where data was removed, causing non-missing entries to be incorrectly tombstoned - Fixed use-after-overwrite: `RemoveMemCachedData`/`RemoveMetaData` were called after `Payload` was overwritten on cache put, leaking stale data - Fixed incorrect retry sleep formula (`100 - (3 - RetriesLeft) * 100` always produced the same or negative value; corrected to `(3 - RetriesLeft) * 100`) - Fixed broken `break` missing from sidecar file read loop, causing reads past valid data - Fixed missing format argument in three `ZEN_WARN`/`ZEN_ERROR` log calls (format string had `{}` placeholders with no corresponding argument, or vice versa) - Fixed elapsed timer being accumulated inside the wrong scope in `HandleRpcGetCacheRecords` - Fixed test asserting against unserialized `RecordPolicy` instead of the deserialized `Loaded` copy - Initialized `AbortFlag`/`PauseFlag` atomics at declaration (UB if read before first write) ## Build store (buildstore.cpp / buildstore.h) - Fixed wrong variable used in warning log: used loop index `ResultIndex` instead of `Index`/`MetaLocationResultIndexes[Index]`, logging wrong hash values - Fixed `sizeof(AccessTimesHeader)` used instead of `sizeof(AccessTimeRecord)` when advancing write offset, corrupting the access times file if the sizes differ - Initialized `m_LastAccessTimeUpdateCount` atomic member (was uninitialized) - Changed map iteration loops to use `const auto&` to avoid unnecessary copies ## Project store (projectstore.cpp / projectstore.h) - Fixed wrong iterator dereferenced in `IterateChunks`: used `ChunkIt->second` (from a different map lookup) instead of `MetaIt->second` - Fixed wrong assert variable: `Sizes[Index]` should be `RawSizes[Index]` - Fixed `MakeTombstone`/`IsTombstone` inconsistency: `MakeTombstone` was zeroing `OpLsn` but `IsTombstone` checks `OpLsn.Number != 0`; tombstone creation now preserves `OpLsn` - Fixed uninitialized `InvalidEntries` counter - Fixed format string mismatch in warning log - Initialized `AbortFlag`/`PauseFlag` atomics; changed map iteration to `const auto&` ## Workspaces (workspaces.cpp) - Fixed missing alias registration when a workspace share is updated: alias was deleted but never re-inserted - Fixed integer overflow in range clamping: `(RequestedOffset + RequestedSize) > Size` could wrap; corrected to `RequestedSize > Size - RequestedOffset` - Changed map iteration loops to `const auto&` ## CAS subsystem (cas.cpp, caslog.cpp, compactcas.cpp, filecas.cpp) - Fixed `IterateChunks` passing original `Payload` buffer instead of the modified `Chunk` buffer (content type was set on the copy but the original was sent to the callback) - Fixed invalid `std::future::get()` call on default-constructed futures - Fixed sign-comparison in `CasLogFile::Replay` loop (`int i` vs `size_t`) - Changed `CasLogFile::IsValid` and `Open` to take `const std::filesystem::path&` instead of by value - Fixed format string in `~CasContainerStrategy` error log ## Remote store (zenremotestore) - Fixed `FolderContent::operator==` always returning true: loop variable `PathCount` was initialized to 0 instead of `Paths.size()` - Fixed `GetChunkIndexForRawHash` looking up from wrong map (`RawHashToSequenceIndex` instead of `ChunkHashToChunkIndex`) - Fixed double-counted `UniqueSequencesFound` stat (incremented in both branches of an if/else) - Fixed `RawSize` sentinel value truncation: `(uint32_t)-1` assigned to a `uint64_t` field; corrected to `(uint64_t)-1` - Initialized uninitialized atomic and struct members across `buildstorageoperations.h`, `chunkblock.h`, and `remoteprojectstore.h`
Diffstat (limited to 'src/zenstore/compactcas.cpp')
-rw-r--r--src/zenstore/compactcas.cpp16
1 files changed, 8 insertions, 8 deletions
diff --git a/src/zenstore/compactcas.cpp b/src/zenstore/compactcas.cpp
index 21411df59..b09892687 100644
--- a/src/zenstore/compactcas.cpp
+++ b/src/zenstore/compactcas.cpp
@@ -153,7 +153,7 @@ CasContainerStrategy::~CasContainerStrategy()
}
catch (const std::exception& Ex)
{
- ZEN_ERROR("~CasContainerStrategy failed with: ", Ex.what());
+ ZEN_ERROR("~CasContainerStrategy failed with: {}", Ex.what());
}
m_Gc.RemoveGcReferenceStore(*this);
m_Gc.RemoveGcStorage(this);
@@ -440,9 +440,9 @@ CasContainerStrategy::IterateChunks(std::span<const IoHash> ChunkHas
return true;
}
- std::atomic<bool> AbortFlag;
+ std::atomic<bool> AbortFlag{false};
{
- std::atomic<bool> PauseFlag;
+ std::atomic<bool> PauseFlag{false};
ParallelWork Work(AbortFlag, PauseFlag, WorkerThreadPool::EMode::DisableBacklog);
try
{
@@ -559,8 +559,8 @@ CasContainerStrategy::ScrubStorage(ScrubContext& Ctx)
std::vector<BlockStoreLocation> ChunkLocations;
std::vector<IoHash> ChunkIndexToChunkHash;
- std::atomic<bool> Abort;
- std::atomic<bool> Pause;
+ std::atomic<bool> Abort{false};
+ std::atomic<bool> Pause{false};
ParallelWork Work(Abort, Pause, WorkerThreadPool::EMode::DisableBacklog);
try
@@ -1007,7 +1007,7 @@ CasContainerStrategy::CompactIndex(RwLock::ExclusiveLockScope&)
std::vector<BlockStoreDiskLocation> Locations;
Locations.reserve(EntryCount);
LocationMap.reserve(EntryCount);
- for (auto It : m_LocationMap)
+ for (const auto& It : m_LocationMap)
{
size_t EntryIndex = Locations.size();
Locations.push_back(m_Locations[It.second]);
@@ -1106,7 +1106,7 @@ CasContainerStrategy::MakeIndexSnapshot(bool ResetLog)
{
// This is non-critical, it only means that we will replay the events of the log over the snapshot - inefficent but in
// the end it will be the same result
- ZEN_WARN("Snapshot failed to clean log file '{}', reason: '{}'", LogPath, IndexPath, Ec.message());
+ ZEN_WARN("Snapshot failed to clean log file '{}', reason: '{}'", LogPath, Ec.message());
}
m_CasLog.Open(LogPath, CasLogFile::Mode::kWrite);
}
@@ -1136,7 +1136,7 @@ CasContainerStrategy::ReadIndexFile(const std::filesystem::path& IndexPath, uint
uint64_t Size = ObjectIndexFile.FileSize();
if (Size >= sizeof(CasDiskIndexHeader))
{
- uint64_t ExpectedEntryCount = (Size - sizeof(sizeof(CasDiskIndexHeader))) / sizeof(CasDiskIndexEntry);
+ uint64_t ExpectedEntryCount = (Size - sizeof(CasDiskIndexHeader)) / sizeof(CasDiskIndexEntry);
CasDiskIndexHeader Header;
ObjectIndexFile.Read(&Header, sizeof(Header), 0);
if ((Header.Magic == CasDiskIndexHeader::ExpectedMagic) && (Header.Version == CasDiskIndexHeader::CurrentVersion) &&