Skip to content

Commit

Permalink
Fix or update references to pwbug/387
Browse files Browse the repository at this point in the history
- Fix all references to pwbug/387 in tests by checking the statuses with
  ASSERT_EQ.
- Update other references to pwbug/387 to refer to the new bug
  b/242598609.

Fixes: b/242050869
Change-Id: I07f5e33dc8149382c9103511e6bcc7e7c51eea63
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/106652
Pigweed-Auto-Submit: Wyatt Hepler <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
Reviewed-by: Rob Mohr <[email protected]>
  • Loading branch information
255 authored and CQ Bot Account committed Aug 15, 2022
1 parent 93a7ca6 commit d4ab5e2
Show file tree
Hide file tree
Showing 66 changed files with 331 additions and 450 deletions.
39 changes: 13 additions & 26 deletions pw_allocator/block_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,10 @@ TEST(Block, CanSplitMidBlock) {
EXPECT_EQ(Block::Init(span(bytes, kN), &block), OkStatus());

Block* block2 = nullptr;
block->Split(kSplit1, &block2)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), block->Split(kSplit1, &block2));

Block* block3 = nullptr;
block->Split(kSplit2, &block3)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), block->Split(kSplit2, &block3));

EXPECT_EQ(block->Next(), block3);
EXPECT_EQ(block3->Next(), block2);
Expand Down Expand Up @@ -284,12 +282,10 @@ TEST(Block, CanMergeWithNextBlock) {
EXPECT_EQ(Block::Init(span(bytes, kN), &block), OkStatus());

Block* block2 = nullptr;
block->Split(kSplit1, &block2)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), block->Split(kSplit1, &block2));

Block* block3 = nullptr;
block->Split(kSplit2, &block3)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), block->Split(kSplit2, &block3));

EXPECT_EQ(block3->MergeNext(), OkStatus());

Expand All @@ -314,8 +310,7 @@ TEST(Block, CannotMergeWithFirstOrLastBlock) {
EXPECT_EQ(Block::Init(span(bytes, kN), &block), OkStatus());

Block* next_block = nullptr;
block->Split(512, &next_block)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), block->Split(512, &next_block));

EXPECT_EQ(next_block->MergeNext(), Status::OutOfRange());
EXPECT_EQ(block->MergePrev(), Status::OutOfRange());
Expand All @@ -331,8 +326,7 @@ TEST(Block, CannotMergeUsedBlock) {
EXPECT_EQ(Block::Init(span(bytes, kN), &block), OkStatus());

Block* next_block = nullptr;
block->Split(512, &next_block)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), block->Split(512, &next_block));

block->MarkUsed();
EXPECT_EQ(block->MergeNext(), Status::FailedPrecondition());
Expand All @@ -347,12 +341,10 @@ TEST(Block, CanCheckValidBlock) {
EXPECT_EQ(Block::Init(span(bytes, kN), &first_block), OkStatus());

Block* second_block = nullptr;
first_block->Split(512, &second_block)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), first_block->Split(512, &second_block));

Block* third_block = nullptr;
second_block->Split(256, &third_block)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), second_block->Split(256, &third_block));

EXPECT_EQ(first_block->IsValid(), true);
EXPECT_EQ(second_block->IsValid(), true);
Expand All @@ -367,16 +359,13 @@ TEST(Block, CanCheckInalidBlock) {
EXPECT_EQ(Block::Init(span(bytes, kN), &first_block), OkStatus());

Block* second_block = nullptr;
first_block->Split(512, &second_block)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), first_block->Split(512, &second_block));

Block* third_block = nullptr;
second_block->Split(256, &third_block)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), second_block->Split(256, &third_block));

Block* fourth_block = nullptr;
third_block->Split(128, &fourth_block)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), third_block->Split(128, &fourth_block));

std::byte* next_ptr = reinterpret_cast<std::byte*>(first_block);
memcpy(next_ptr, second_block, sizeof(void*));
Expand Down Expand Up @@ -408,12 +397,10 @@ TEST(Block, CanPoisonBlock) {
EXPECT_EQ(Block::Init(span(bytes, kN), &first_block), OkStatus());

Block* second_block = nullptr;
first_block->Split(512, &second_block)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), first_block->Split(512, &second_block));

Block* third_block = nullptr;
second_block->Split(256, &third_block)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), second_block->Split(256, &third_block));

EXPECT_EQ(first_block->IsValid(), true);
EXPECT_EQ(second_block->IsValid(), true);
Expand Down
16 changes: 8 additions & 8 deletions pw_allocator/freelist_heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ FreeListHeap::FreeListHeap(span<std::byte> region, FreeList& freelist)
"Failed to initialize FreeListHeap region; misaligned or too small");

freelist_.AddChunk(BlockToSpan(block))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly

region_ = region;
heap_stats_.total_bytes = region.size();
Expand All @@ -44,7 +44,7 @@ void* FreeListHeap::Allocate(size_t size) {
return nullptr;
}
freelist_.RemoveChunk(chunk)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly

Block* chunk_block = Block::FromUsableSpace(chunk.data());

Expand All @@ -55,7 +55,7 @@ void* FreeListHeap::Allocate(size_t size) {
auto status = chunk_block->Split(size, &leftover);
if (status == PW_STATUS_OK) {
freelist_.AddChunk(BlockToSpan(leftover))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
}

chunk_block->MarkUsed();
Expand Down Expand Up @@ -96,23 +96,23 @@ void FreeListHeap::Free(void* ptr) {
if (prev != nullptr && !prev->Used()) {
// Remove from freelist and merge
freelist_.RemoveChunk(BlockToSpan(prev))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
chunk_block->MergePrev()
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly

// chunk_block is now invalid; prev now encompasses it.
chunk_block = prev;
}

if (next != nullptr && !next->Used()) {
freelist_.RemoveChunk(BlockToSpan(next))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
chunk_block->MergeNext()
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
}
// Add back to the freelist
freelist_.AddChunk(BlockToSpan(chunk_block))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly

heap_stats_.bytes_allocated -= size_freed;
heap_stats_.cumulative_freed += size_freed;
Expand Down
39 changes: 13 additions & 26 deletions pw_allocator/freelist_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ TEST(FreeList, CanRetrieveAddedMemberForSmallerSize) {

byte data[kN] = {std::byte(0)};

list.AddChunk(span(data, kN))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), list.AddChunk(span(data, kN)));
auto item = list.FindChunk(kN / 2);
EXPECT_EQ(item.size(), kN);
EXPECT_EQ(item.data(), data);
Expand All @@ -68,8 +67,7 @@ TEST(FreeList, CanRemoveItem) {

byte data[kN] = {std::byte(0)};

list.AddChunk(span(data, kN))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), list.AddChunk(span(data, kN)));
auto status = list.RemoveChunk(span(data, kN));
EXPECT_EQ(status, OkStatus());

Expand All @@ -85,10 +83,8 @@ TEST(FreeList, FindReturnsSmallestChunk) {
byte data1[kN1] = {std::byte(0)};
byte data2[kN2] = {std::byte(0)};

list.AddChunk(span(data1, kN1))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
list.AddChunk(span(data2, kN2))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), list.AddChunk(span(data1, kN1)));
ASSERT_EQ(OkStatus(), list.AddChunk(span(data2, kN2)));

auto chunk = list.FindChunk(kN1 / 2);
EXPECT_EQ(chunk.size(), kN1);
Expand All @@ -114,10 +110,8 @@ TEST(FreeList, FindReturnsCorrectChunkInSameBucket) {
byte data2[kN2] = {std::byte(0)};

// List should now be 257 -> 512 -> NULL
list.AddChunk(span(data1, kN1))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
list.AddChunk(span(data2, kN2))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), list.AddChunk(span(data1, kN1)));
ASSERT_EQ(OkStatus(), list.AddChunk(span(data2, kN2)));

auto chunk = list.FindChunk(kN2 + 1);
EXPECT_EQ(chunk.size(), kN1);
Expand All @@ -136,10 +130,8 @@ TEST(FreeList, FindCanMoveUpThroughBuckets) {
// List should now be:
// bkt[3] (257 bytes up to 512 bytes) -> 257 -> NULL
// bkt[4] (513 bytes up to 1024 bytes) -> 513 -> NULL
list.AddChunk(span(data1, kN1))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
list.AddChunk(span(data2, kN2))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), list.AddChunk(span(data1, kN1)));
ASSERT_EQ(OkStatus(), list.AddChunk(span(data2, kN2)));

// Request a 300 byte chunk. This should return the 513 byte one
auto chunk = list.FindChunk(kN1 + 1);
Expand All @@ -153,8 +145,7 @@ TEST(FreeList, RemoveUnknownChunkReturnsNotFound) {
byte data[kN] = {std::byte(0)};
byte data2[kN] = {std::byte(0)};

list.AddChunk(span(data, kN))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), list.AddChunk(span(data, kN)));
auto status = list.RemoveChunk(span(data2, kN));
EXPECT_EQ(status, Status::NotFound());
}
Expand All @@ -166,17 +157,13 @@ TEST(FreeList, CanStoreMultipleChunksPerBucket) {
byte data1[kN] = {std::byte(0)};
byte data2[kN] = {std::byte(0)};

list.AddChunk(span(data1, kN))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
list.AddChunk(span(data2, kN))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), list.AddChunk(span(data1, kN)));
ASSERT_EQ(OkStatus(), list.AddChunk(span(data2, kN)));

auto chunk1 = list.FindChunk(kN);
list.RemoveChunk(chunk1)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), list.RemoveChunk(chunk1));
auto chunk2 = list.FindChunk(kN);
list.RemoveChunk(chunk2)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), list.RemoveChunk(chunk2));

// Ordering of the chunks doesn't matter
EXPECT_TRUE(chunk1.data() != chunk2.data());
Expand Down
2 changes: 1 addition & 1 deletion pw_assert_basic/basic_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ static const char* kCrashBanner[] = {

static void WriteLine(const std::string_view& s) {
pw::sys_io::WriteLine(s)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
}

typedef pw::StringBuffer<150> Buffer;
Expand Down
6 changes: 3 additions & 3 deletions pw_blob_store/blob_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ Status BlobStore::LoadMetadata() {
metadata.v1_metadata.checksum)
.ok()) {
PW_LOG_ERROR("BlobStore init - Invalidating blob with invalid checksum");
Invalidate().IgnoreError(); // TODO(pwbug/387): Handle Status properly
Invalidate().IgnoreError(); // TODO(b/242598609): Handle Status properly
return Status::DataLoss();
}

Expand Down Expand Up @@ -120,7 +120,7 @@ Status BlobStore::OpenWrite() {
writer_open_ = true;

// Clear any existing contents.
Invalidate().IgnoreError(); // TODO(pwbug/387): Handle Status properly
Invalidate().IgnoreError(); // TODO(b/242598609): Handle Status properly

return OkStatus();
}
Expand Down Expand Up @@ -453,7 +453,7 @@ Status BlobStore::Erase() {

// If any writes have been performed, reset the state.
if (flash_address_ != 0) {
Invalidate().IgnoreError(); // TODO(pwbug/387): Handle Status properly
Invalidate().IgnoreError(); // TODO(b/242598609): Handle Status properly
}

PW_TRY(partition_.Erase());
Expand Down
12 changes: 4 additions & 8 deletions pw_blob_store/blob_store_chunk_write_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,18 @@ class BlobStoreChunkTest : public ::testing::Test {
BlobStoreChunkTest() : flash_(kFlashAlignment), partition_(&flash_) {}

void InitFlashTo(span<const std::byte> contents) {
partition_.Erase()
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), partition_.Erase());
std::memcpy(flash_.buffer().data(), contents.data(), contents.size());
}

void InitSourceBufferToRandom(uint64_t seed) {
partition_.Erase()
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), partition_.Erase());
random::XorShiftStarRng64 rng(seed);
rng.Get(source_buffer_)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), rng.Get(source_buffer_).status());
}

void InitSourceBufferToFill(char fill) {
partition_.Erase()
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), partition_.Erase());
std::memset(source_buffer_.data(), fill, source_buffer_.size());
}

Expand Down
7 changes: 3 additions & 4 deletions pw_blob_store/blob_store_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ class BlobStoreTest : public ::testing::Test {
BlobStoreTest() : flash_(kFlashAlignment), partition_(&flash_) {}

void InitFlashTo(span<const std::byte> contents) {
partition_.Erase()
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(), partition_.Erase());
std::memcpy(flash_.buffer().data(), contents.data(), contents.size());
}

Expand All @@ -54,8 +53,8 @@ class BlobStoreTest : public ::testing::Test {
std::memset(source_buffer_.data(),
static_cast<int>(flash_.erased_memory_content()),
source_buffer_.size());
rng.Get(span(source_buffer_).first(init_size_bytes))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(),
rng.Get(span(source_buffer_).first(init_size_bytes)).status());
}

void InitSourceBufferToFill(char fill,
Expand Down
4 changes: 2 additions & 2 deletions pw_blob_store/flat_file_system_entry_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class FlatFileSystemBlobStoreEntryTest : public ::testing::Test {
std::memset(source_buffer_.data(),
static_cast<int>(flash_.erased_memory_content()),
source_buffer_.size());
rng.Get(span(source_buffer_).first(init_size_bytes))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
ASSERT_EQ(OkStatus(),
rng.Get(span(source_buffer_).first(init_size_bytes)).status());
}

// Fill the source buffer with random pattern based on given seed, written to
Expand Down
2 changes: 1 addition & 1 deletion pw_blob_store/public/pw_blob_store/blob_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class BlobStore {
BlobWriter& operator=(const BlobWriter&) = delete;
~BlobWriter() override {
if (open_) {
Close().IgnoreError(); // TODO(pwbug/387): Handle Status properly
Close().IgnoreError(); // TODO(b/242598609): Handle Status properly
}
}

Expand Down
12 changes: 6 additions & 6 deletions pw_file/flat_file_system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ void FlatFileSystemService::EnumerateAllFiles(RawServerWriter& writer) {
Status write_status = writer.Write(encoder);
if (!write_status.ok()) {
writer.Finish(write_status)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
return;
}
}
writer.Finish(OkStatus())
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
}

void FlatFileSystemService::List(ConstByteSpan request,
Expand All @@ -95,28 +95,28 @@ void FlatFileSystemService::List(ConstByteSpan request,
if (!decoder.ReadString(&file_name_view).ok() ||
file_name_view.length() == 0) {
writer.Finish(Status::DataLoss())
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
return;
}

// Find and enumerate the file requested.
Result<Entry*> result = FindFile(file_name_view);
if (!result.ok()) {
writer.Finish(result.status())
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
return;
}

pw::file::ListResponse::MemoryEncoder encoder(encoding_buffer_);
Status proto_encode_status = EnumerateFile(*result.value(), encoder);
if (!proto_encode_status.ok()) {
writer.Finish(proto_encode_status)
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
return;
}

writer.Finish(writer.Write(encoder))
.IgnoreError(); // TODO(pwbug/387): Handle Status properly
.IgnoreError(); // TODO(b/242598609): Handle Status properly
return;
}

Expand Down
Loading

0 comments on commit d4ab5e2

Please sign in to comment.