From bad6d27519d3a4679f0d4bb93090a6206017ad5f Mon Sep 17 00:00:00 2001 From: Wyatt Hepler Date: Wed, 16 Feb 2022 07:15:07 -0800 Subject: pw_status: Enforce [[nodiscard]] on pw::Status in upstream Pigweed - Enable the [[nodiscard]] flag for the pw_strict_* toolchains. - Check unused Status or call IgnoreError() and flag with pwbug/387. Change-Id: Ibef43c09b29a803bc3be17f81e13639d5f908cb2 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/77561 Reviewed-by: Alexei Frolov Pigweed-Auto-Submit: Wyatt Hepler Reviewed-by: Keir Mierle Commit-Queue: Auto-Submit --- pw_log_rpc/log_filter_service_test.cc | 21 +++++++++++---------- pw_log_rpc/log_service_test.cc | 18 +++++++++++------- pw_log_rpc/rpc_log_drain.cc | 5 +++-- 3 files changed, 25 insertions(+), 19 deletions(-) (limited to 'pw_log_rpc') diff --git a/pw_log_rpc/log_filter_service_test.cc b/pw_log_rpc/log_filter_service_test.cc index c42777974..3562c4054 100644 --- a/pw_log_rpc/log_filter_service_test.cc +++ b/pw_log_rpc/log_filter_service_test.cc @@ -59,7 +59,7 @@ class FilterServiceTest : public ::testing::Test { TEST_F(FilterServiceTest, GetFilterIds) { PW_RAW_TEST_METHOD_CONTEXT(FilterService, ListFilterIds, 1) context(filter_map_); - context.call({}); + ASSERT_EQ(OkStatus(), context.call({}).status()); ASSERT_TRUE(context.done()); ASSERT_EQ(context.responses().size(), 1u); protobuf::Decoder decoder(context.responses()[0]); @@ -79,7 +79,7 @@ TEST_F(FilterServiceTest, GetFilterIds) { FilterMap empty_filter_map({}); PW_RAW_TEST_METHOD_CONTEXT(FilterService, ListFilterIds, 1) no_filter_context(empty_filter_map); - no_filter_context.call({}); + ASSERT_EQ(OkStatus(), no_filter_context.call({}).status()); ASSERT_TRUE(no_filter_context.done()); ASSERT_EQ(no_filter_context.responses().size(), 1u); protobuf::Decoder no_filter_decoder(no_filter_context.responses()[0]); @@ -168,7 +168,7 @@ TEST_F(FilterServiceTest, SetFilterRules) { PW_RAW_TEST_METHOD_CONTEXT(FilterService, SetFilter, 1) context(filter_map_); - context.call(request.value()); + ASSERT_EQ(OkStatus(), context.call(request.value()).status()); size_t i = 0; for (const auto& rule : filters_[0].rules()) { @@ -213,7 +213,7 @@ TEST_F(FilterServiceTest, SetFilterRulesWhenUsedByDrain) { PW_RAW_TEST_METHOD_CONTEXT(FilterService, SetFilter, 1) context(filter_map_); - context.call(request.value()); + ASSERT_EQ(OkStatus(), context.call(request.value()).status()); size_t i = 0; for (const auto& rule : filter.rules()) { @@ -223,7 +223,7 @@ TEST_F(FilterServiceTest, SetFilterRulesWhenUsedByDrain) { // An empty request should not modify the filter. PW_RAW_TEST_METHOD_CONTEXT(FilterService, SetFilter, 1) context_no_filter(filter_map_); - context_no_filter.call({}); + EXPECT_EQ(Status::OutOfRange(), context_no_filter.call({}).status()); i = 0; for (const auto& rule : filter.rules()) { VerifyRule(rule, new_filter_rules[i++]); @@ -266,7 +266,8 @@ TEST_F(FilterServiceTest, SetFilterRulesWhenUsedByDrain) { ASSERT_EQ(second_filter_request.status(), OkStatus()); PW_RAW_TEST_METHOD_CONTEXT(FilterService, SetFilter, 1) context_new_filter(filter_map_); - context_new_filter.call(second_filter_request.value()); + ASSERT_EQ(OkStatus(), + context_new_filter.call(second_filter_request.value()).status()); i = 0; for (const auto& rule : filter.rules()) { @@ -331,9 +332,9 @@ TEST_F(FilterServiceTest, GetFilterRules) { std::byte request_buffer[64]; log::GetFilterRequest::MemoryEncoder encoder(request_buffer); - encoder.WriteFilterId(filter_id1_); + ASSERT_EQ(OkStatus(), encoder.WriteFilterId(filter_id1_)); const auto request = ConstByteSpan(encoder); - context.call(request); + ASSERT_EQ(OkStatus(), context.call(request).status()); ASSERT_TRUE(context.done()); ASSERT_EQ(context.responses().size(), 1u); @@ -353,7 +354,7 @@ TEST_F(FilterServiceTest, GetFilterRules) { PW_RAW_TEST_METHOD_CONTEXT(FilterService, GetFilter, 1) context2(filter_map_); - context2.call(request); + ASSERT_EQ(OkStatus(), context2.call(request).status()); ASSERT_EQ(context2.responses().size(), 1u); protobuf::Decoder decoder2(context2.responses()[0]); VerifyFilterRules(decoder2, rules1_); @@ -368,7 +369,7 @@ TEST_F(FilterServiceTest, GetFilterRules) { PW_RAW_TEST_METHOD_CONTEXT(FilterService, GetFilter, 1) context3(filter_map_); - context3.call(request); + ASSERT_EQ(OkStatus(), context3.call(request).status()); ASSERT_EQ(context3.responses().size(), 1u); protobuf::Decoder decoder3(context3.responses()[0]); VerifyFilterRules(decoder3, rules1_); diff --git a/pw_log_rpc/log_service_test.cc b/pw_log_rpc/log_service_test.cc index 48aa32411..3afb5f482 100644 --- a/pw_log_rpc/log_service_test.cc +++ b/pw_log_rpc/log_service_test.cc @@ -209,7 +209,7 @@ TEST_F(LogServiceTest, StartAndEndStream) { // Not done until the stream is finished. ASSERT_FALSE(context.done()); - active_drain.Close(); + EXPECT_EQ(OkStatus(), active_drain.Close()); ASSERT_TRUE(context.done()); EXPECT_EQ(context.status(), OkStatus()); @@ -258,7 +258,7 @@ TEST_F(LogServiceTest, HandleDropped) { // Request logs. context.call(rpc_request_buffer); EXPECT_EQ(active_drain.Flush(encoding_buffer_), OkStatus()); - active_drain.Close(); + EXPECT_EQ(OkStatus(), active_drain.Close()); ASSERT_EQ(context.status(), OkStatus()); // There is at least 1 response with multiple log entries packed. ASSERT_GE(context.responses().size(), 1u); @@ -309,18 +309,21 @@ TEST_F(LogServiceTest, HandleDroppedBetweenFilteredOutLogs) { // Force a drop entry in between entries that will be filtered out. for (size_t i = 1; i < total_entries; ++i) { - AddLogEntry(kMessage, kSampleMetadata, kSampleTimestamp); + ASSERT_EQ( + OkStatus(), + AddLogEntry(kMessage, kSampleMetadata, kSampleTimestamp).status()); multisink_.HandleDropped(1); } // Add message that won't be filtered out. constexpr auto metadata = log_tokenized::Metadata::Set(); - AddLogEntry(kMessage, metadata, kSampleTimestamp); + ASSERT_EQ(OkStatus(), + AddLogEntry(kMessage, metadata, kSampleTimestamp).status()); // Request logs. context.call(rpc_request_buffer); EXPECT_EQ(active_drain.Flush(encoding_buffer_), OkStatus()); - active_drain.Close(); + EXPECT_EQ(OkStatus(), active_drain.Close()); ASSERT_EQ(context.status(), OkStatus()); // There is at least 1 response with multiple log entries packed. ASSERT_GE(context.responses().size(), 1u); @@ -359,7 +362,8 @@ TEST_F(LogServiceTest, HandleSmallLogEntryBuffer) { const uint32_t total_drop_count = total_entries - 1; AddLogEntries( total_entries - 1, kLongMessage, kSampleMetadata, kSampleTimestamp); - AddLogEntry(kMessage, kSampleMetadata, kSampleTimestamp); + EXPECT_EQ(OkStatus(), + AddLogEntry(kMessage, kSampleMetadata, kSampleTimestamp).status()); // Request logs. context.call(rpc_request_buffer); @@ -442,7 +446,7 @@ TEST_F(LogServiceTest, LargeLogEntry) { context.set_channel_id(drain_channel_id); context.call(rpc_request_buffer); ASSERT_EQ(active_drain.Flush(encoding_buffer_), OkStatus()); - active_drain.Close(); + EXPECT_EQ(OkStatus(), active_drain.Close()); ASSERT_EQ(context.status(), OkStatus()); ASSERT_EQ(context.responses().size(), 1u); diff --git a/pw_log_rpc/rpc_log_drain.cc b/pw_log_rpc/rpc_log_drain.cc index 41e092fbc..d70b09967 100644 --- a/pw_log_rpc/rpc_log_drain.cc +++ b/pw_log_rpc/rpc_log_drain.cc @@ -34,7 +34,7 @@ Result CreateEncodedDropMessage( uint32_t drop_count, ByteSpan encoded_drop_message_buffer) { // Encode message in protobuf. log::LogEntry::MemoryEncoder encoder(encoded_drop_message_buffer); - encoder.WriteDropped(drop_count); + encoder.WriteDropped(drop_count).IgnoreError(); PW_TRY(encoder.status()); return ConstByteSpan(encoder); } @@ -102,7 +102,8 @@ RpcLogDrain::LogDrainState RpcLogDrain::SendLogs(size_t max_num_bundles, continue; } - encoder.WriteFirstEntrySequenceId(sequence_id_); + encoder.WriteFirstEntrySequenceId(sequence_id_) + .IgnoreError(); // TODO(pwbug/387): Handle Status properly sequence_id_ += packed_entry_count; const Status status = server_writer_.Write(encoder); sent_bundle_count++; -- cgit v1.2.3