Skip to content

Commit

Permalink
pw_status: Don't silently discard status returns
Browse files Browse the repository at this point in the history
This fixes all instances in upstream Pigweed, but doesn't turn on
enforcement yet (I need to fix downstream Bazel projects separately
first).

Bug: 357090965
Change-Id: I10a090c6bbd3165774b2675410ce3e700b6a32ea
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/227277
Lint: Lint 🤖 <[email protected]>
Reviewed-by: Wyatt Hepler <[email protected]>
Commit-Queue: Ted Pudlik <[email protected]>
  • Loading branch information
tpudlik authored and CQ Bot Account committed Aug 2, 2024
1 parent 7794881 commit 474eecb
Show file tree
Hide file tree
Showing 20 changed files with 139 additions and 107 deletions.
2 changes: 1 addition & 1 deletion pw_channel/stream_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ Poll<Status> StreamChannel::DoPendReadyToWrite(Context&) { return OkStatus(); }

pw::Result<pw::channel::WriteToken> StreamChannel::DoWrite(
pw::multibuf::MultiBuf&& data) {
write_state_.SendData(std::move(data));
PW_TRY(write_state_.SendData(std::move(data)));
const uint32_t token = write_token_++;
return CreateWriteToken(token);
}
Expand Down
8 changes: 5 additions & 3 deletions pw_cpu_exception_cortex_m/snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ Status CaptureMainStack(
thread_state = thread::proto::pwpb::ThreadState::Enum::RUNNING;
PW_LOG_DEBUG("Thread state: RUNNING");
}
encoder.WriteState(thread_state);
encoder.WriteName(as_bytes(span(std::string_view(thread_name))));
// TODO: https://pwbug.dev/357138093 - Review IgnoreError calls in this file.
encoder.WriteState(thread_state).IgnoreError();
encoder.WriteName(as_bytes(span(std::string_view(thread_name))))
.IgnoreError();

const thread::StackContext thread_ctx = {
.thread_name = thread_name,
Expand All @@ -77,7 +79,7 @@ Status SnapshotCpuState(
{
pwpb::ArmV7mCpuState::StreamEncoder cpu_state_encoder =
snapshot_encoder.GetArmv7mCpuStateEncoder();
DumpCpuStateProto(cpu_state_encoder, cpu_state);
DumpCpuStateProto(cpu_state_encoder, cpu_state).IgnoreError();
}
return snapshot_encoder.status();
}
Expand Down
3 changes: 2 additions & 1 deletion pw_crypto/size_report/ecdsa_p256_verify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ int main() {
pw::bloat::BloatThisBinary();

pw::crypto::ecdsa::VerifyP256Signature(
STR_TO_BYTES(PUBKEY), STR_TO_BYTES(DIGEST), STR_TO_BYTES(SIGNATURE));
STR_TO_BYTES(PUBKEY), STR_TO_BYTES(DIGEST), STR_TO_BYTES(SIGNATURE))
.IgnoreError();
return 0;
}
2 changes: 2 additions & 0 deletions pw_hdlc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,11 @@ cc_library(
":default_addresses",
":pw_hdlc",
":rpc_channel_output",
"//pw_assert",
"//pw_log",
"//pw_log_basic:headers",
"//pw_rpc/system_server:facade",
"//pw_status",
"//pw_stream:sys_io_stream",
],
)
Expand Down
13 changes: 6 additions & 7 deletions pw_hdlc/hdlc_sys_io_system_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@

#include <cstddef>

#include "pw_assert/check.h"
#include "pw_hdlc/decoder.h"
#include "pw_hdlc/default_addresses.h"
#include "pw_hdlc/encoded_size.h"
#include "pw_hdlc/rpc_channel.h"
#include "pw_log_basic/log_basic.h"
#include "pw_rpc_system_server/rpc_server.h"
#include "pw_status/try.h"
#include "pw_stream/sys_io_stream.h"

namespace pw::rpc::system_server {
Expand Down Expand Up @@ -47,8 +49,8 @@ void Init() {
// Send log messages to HDLC address 1. This prevents logs from interfering
// with pw_rpc communications.
pw::log_basic::SetOutput([](std::string_view log) {
pw::hdlc::WriteUIFrame(
pw::hdlc::kDefaultLogAddress, as_bytes(span<const char>(log)), writer);
PW_CHECK_OK(pw::hdlc::WriteUIFrame(
pw::hdlc::kDefaultLogAddress, as_bytes(span<const char>(log)), writer));
});
}

Expand All @@ -63,14 +65,11 @@ Status Start() {

while (true) {
std::byte byte;
Status ret_val = pw::sys_io::ReadByte(&byte);
if (!ret_val.ok()) {
return ret_val;
}
PW_TRY(pw::sys_io::ReadByte(&byte));
if (auto result = decoder.Process(byte); result.ok()) {
hdlc::Frame& frame = result.value();
if (frame.address() == hdlc::kDefaultRpcAddress) {
server.ProcessPacket(frame.data());
PW_TRY(server.ProcessPacket(frame.data()));
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions pw_hdlc/size_report/hdlc_size_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ void HdlcSizeReport() {
std::array<std::byte, 100> data;
std::fill(data.begin(), data.end(), std::byte(0));
stream::MemoryWriterBuffer<250> destination;
destination.Write(data);
if (destination.Write(data).ok()) {
PW_LOG_INFO("Began write successfully");
}
if (destination.Write(GetBufferSpan()).ok()) {
PW_LOG_INFO("Wrote successfully");
}
Expand All @@ -82,7 +84,7 @@ void HdlcSizeReport() {
#endif

#ifdef ENABLE_ENCODE
hdlc::WriteUIFrame(123, data, destination);
hdlc::WriteUIFrame(123, data, destination).IgnoreError();
#endif

#ifdef ENABLE_DECODE
Expand Down
4 changes: 3 additions & 1 deletion pw_log_tokenized/base64_over_hdlc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ extern "C" void pw_log_tokenized_HandleLog(
PrefixedBase64Encode(log_buffer, size_bytes);

// HDLC-encode the Base64 string via a SysIoWriter.
// TODO: https://pwbug.dev/357136096 - Maybe don't ignore the error?
hdlc::WriteUIFrame(
kBase64LogHdlcAddress, as_bytes(span(base64_string)), writer);
kBase64LogHdlcAddress, as_bytes(span(base64_string)), writer)
.IgnoreError();
}

} // namespace pw::log_tokenized
16 changes: 8 additions & 8 deletions pw_protobuf/size_report/decoder_incremental.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,30 +43,30 @@ int main() {
while (decoder.Next().ok()) {
switch (decoder.FieldNumber()) {
case 1:
decoder.ReadInt32(&test_int32);
decoder.ReadInt32(&test_int32).IgnoreError();
break;
case 2:
decoder.ReadSint32(&test_sint32);
decoder.ReadSint32(&test_sint32).IgnoreError();
break;
case 3:
decoder.ReadString(&str);
decoder.ReadString(&str).IgnoreError();
break;
case 4:
decoder.ReadFloat(&f);
decoder.ReadFloat(&f).IgnoreError();
break;
case 5:
decoder.ReadDouble(&d);
decoder.ReadDouble(&d).IgnoreError();
break;

// Extra fields over decoder_full.
case 21:
decoder.ReadInt32(&test_int32);
decoder.ReadInt32(&test_int32).IgnoreError();
break;
case 22:
decoder.ReadUint32(&uint);
decoder.ReadUint32(&uint).IgnoreError();
break;
case 23:
decoder.ReadSint32(&test_sint32);
decoder.ReadSint32(&test_sint32).IgnoreError();
break;
}
}
Expand Down
10 changes: 5 additions & 5 deletions pw_protobuf/size_report/decoder_partial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,19 @@ int main() {
while (decoder.Next().ok()) {
switch (decoder.FieldNumber()) {
case 1:
decoder.ReadInt32(&test_int32);
decoder.ReadInt32(&test_int32).IgnoreError();
break;
case 2:
decoder.ReadSint32(&test_sint32);
decoder.ReadSint32(&test_sint32).IgnoreError();
break;
case 3:
decoder.ReadString(&str);
decoder.ReadString(&str).IgnoreError();
break;
case 4:
decoder.ReadFloat(&f);
decoder.ReadFloat(&f).IgnoreError();
break;
case 5:
decoder.ReadDouble(&d);
decoder.ReadDouble(&d).IgnoreError();
break;
}
}
Expand Down
19 changes: 9 additions & 10 deletions pw_protobuf/size_report/oneof_codegen_comparison.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,17 @@ PW_NO_INLINE void BasicEncode() {
volatile bool has_timestamp = true;
volatile bool has_has_value = false;
if (which_key == KeyType::KEY_STRING) {
encoder.WriteString(1, "test");
encoder.WriteString(1, "test").IgnoreError();
} else if (which_key == KeyType::KEY_TOKEN) {
encoder.WriteFixed32(2, 99999);
encoder.WriteFixed32(2, 99999).IgnoreError();
}

if (has_timestamp) {
encoder.WriteInt64(3, 1663003467);
encoder.WriteInt64(3, 1663003467).IgnoreError();
}

if (has_has_value) {
encoder.WriteBool(4, true);
encoder.WriteBool(4, true).IgnoreError();
}

{
Expand Down Expand Up @@ -189,17 +189,17 @@ PW_NO_INLINE void BasicEncode() {
volatile bool has_timestamp = true;
volatile bool has_has_value = false;
if (which_key == KeyType::KEY_STRING) {
encoder.WriteKeyString("test");
encoder.WriteKeyString("test").IgnoreError();
} else if (which_key == KeyType::KEY_TOKEN) {
encoder.WriteKeyToken(99999);
encoder.WriteKeyToken(99999).IgnoreError();
}

if (has_timestamp) {
encoder.WriteTimestamp(1663003467);
encoder.WriteTimestamp(1663003467).IgnoreError();
}

if (has_has_value) {
encoder.WriteHasValue(true);
encoder.WriteHasValue(true).IgnoreError();
}

{
Expand Down Expand Up @@ -334,8 +334,7 @@ PW_NO_INLINE void BasicEncode() {
if (which_key == KeyType::KEY_STRING) {
message.key_string.SetEncoder(
[](ResponseInfo::StreamEncoder& key_string_encoder) -> pw::Status {
key_string_encoder.WriteKeyString("test");
return pw::OkStatus();
return key_string_encoder.WriteKeyString("test");
});
} else if (which_key == KeyType::KEY_TOKEN) {
message.key_token = 99999;
Expand Down
94 changes: 48 additions & 46 deletions pw_protobuf/size_report/proto_bloat.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,14 @@ class FakeMessageEncoder : public protobuf::StreamEncoder {
public:
FakeMessageEncoder(stream::Writer& writer)
: protobuf::StreamEncoder(writer, ByteSpan()) {}
void DoBloat() { Write(ByteSpan(), kFakeTable); }
void DoBloat() { Write(ByteSpan(), kFakeTable).IgnoreError(); }
};

class FakeMessageDecoder : public protobuf::StreamDecoder {
public:
FakeMessageDecoder(stream::Reader& reader)
: protobuf::StreamDecoder(reader) {}
void DoBloat() { Read(ByteSpan(), kFakeTable); }
void DoBloat() { Read(ByteSpan(), kFakeTable).IgnoreError(); }
};

void CodeToSetUpSizeReportEnvironment() {
Expand Down Expand Up @@ -135,8 +135,8 @@ void Dependencies() {
std::array<std::byte, 2> buffer;
stream::NullStream null_stream;
stream::MemoryWriter memory_writer(buffer);
memory_writer.Write(buffer);
null_stream.Write(buffer);
memory_writer.Write(buffer).IgnoreError();
null_stream.Write(buffer).IgnoreError();
stream::MemoryReader memory_reader(buffer);
memory_reader.Read(buffer).IgnoreError();
}
Expand All @@ -145,68 +145,70 @@ void CodeToPullInProtoEncoder() {
std::array<std::byte, 1024> buffer;
protobuf::MemoryEncoder encoder(buffer);

encoder.WriteUint32(1, 1);
encoder.WritePackedUint32(1, GetIntegerArray<uint32_t>());
encoder.WriteRepeatedUint32(1, GetIntegerVector<uint32_t>());
encoder.WriteUint32(1, 1).IgnoreError();
encoder.WritePackedUint32(1, GetIntegerArray<uint32_t>()).IgnoreError();
encoder.WriteRepeatedUint32(1, GetIntegerVector<uint32_t>()).IgnoreError();

encoder.WriteInt32(1, 1);
encoder.WritePackedInt32(1, GetIntegerArray<int32_t>());
encoder.WriteRepeatedInt32(1, GetIntegerVector<int32_t>());
encoder.WriteInt32(1, 1).IgnoreError();
encoder.WritePackedInt32(1, GetIntegerArray<int32_t>()).IgnoreError();
encoder.WriteRepeatedInt32(1, GetIntegerVector<int32_t>()).IgnoreError();

encoder.WriteUint64(1, 1);
encoder.WritePackedUint64(1, GetIntegerArray<uint64_t>());
encoder.WriteRepeatedUint64(1, GetIntegerVector<uint64_t>());
encoder.WriteUint64(1, 1).IgnoreError();
encoder.WritePackedUint64(1, GetIntegerArray<uint64_t>()).IgnoreError();
encoder.WriteRepeatedUint64(1, GetIntegerVector<uint64_t>()).IgnoreError();

encoder.WriteInt64(1, 1);
encoder.WritePackedInt64(1, GetIntegerArray<int64_t>());
encoder.WriteRepeatedInt64(1, GetIntegerVector<int64_t>());
encoder.WriteInt64(1, 1).IgnoreError();
encoder.WritePackedInt64(1, GetIntegerArray<int64_t>()).IgnoreError();
encoder.WriteRepeatedInt64(1, GetIntegerVector<int64_t>()).IgnoreError();

encoder.WriteSint32(1, 1);
encoder.WritePackedSint32(1, GetIntegerArray<int32_t>());
encoder.WriteRepeatedSint32(1, GetIntegerVector<int32_t>());
encoder.WriteSint32(1, 1).IgnoreError();
encoder.WritePackedSint32(1, GetIntegerArray<int32_t>()).IgnoreError();
encoder.WriteRepeatedSint32(1, GetIntegerVector<int32_t>()).IgnoreError();

encoder.WriteSint64(1, 1);
encoder.WritePackedSint64(1, GetIntegerArray<int64_t>());
encoder.WriteRepeatedSint64(1, GetIntegerVector<int64_t>());
encoder.WriteSint64(1, 1).IgnoreError();
encoder.WritePackedSint64(1, GetIntegerArray<int64_t>()).IgnoreError();
encoder.WriteRepeatedSint64(1, GetIntegerVector<int64_t>()).IgnoreError();

encoder.WriteFixed32(1, 1);
encoder.WritePackedFixed32(1, GetIntegerArray<uint32_t>());
encoder.WriteRepeatedFixed32(1, GetIntegerVector<uint32_t>());
encoder.WriteFixed32(1, 1).IgnoreError();
encoder.WritePackedFixed32(1, GetIntegerArray<uint32_t>()).IgnoreError();
encoder.WriteRepeatedFixed32(1, GetIntegerVector<uint32_t>()).IgnoreError();

encoder.WriteFixed64(1, 1);
encoder.WritePackedFixed64(1, GetIntegerArray<uint64_t>());
encoder.WriteRepeatedFixed64(1, GetIntegerVector<uint64_t>());
encoder.WriteFixed64(1, 1).IgnoreError();
encoder.WritePackedFixed64(1, GetIntegerArray<uint64_t>()).IgnoreError();
encoder.WriteRepeatedFixed64(1, GetIntegerVector<uint64_t>()).IgnoreError();

encoder.WriteSfixed32(1, 1);
encoder.WritePackedSfixed32(1, GetIntegerArray<int32_t>());
encoder.WriteRepeatedSfixed32(1, GetIntegerVector<int32_t>());
encoder.WriteSfixed32(1, 1).IgnoreError();
encoder.WritePackedSfixed32(1, GetIntegerArray<int32_t>()).IgnoreError();
encoder.WriteRepeatedSfixed32(1, GetIntegerVector<int32_t>()).IgnoreError();

encoder.WriteSfixed64(1, 1);
encoder.WritePackedSfixed64(1, GetIntegerArray<int64_t>());
encoder.WriteRepeatedSfixed64(1, GetIntegerVector<int64_t>());
encoder.WriteSfixed64(1, 1).IgnoreError();
encoder.WritePackedSfixed64(1, GetIntegerArray<int64_t>()).IgnoreError();
encoder.WriteRepeatedSfixed64(1, GetIntegerVector<int64_t>()).IgnoreError();

{
protobuf::StreamEncoder child = encoder.GetNestedEncoder(0xc01dfee7);

child.WriteFloat(234, 3.14f);
child.WritePackedFloat(234, GetFloatArray());
child.WriteRepeatedFloat(234, GetFloatVector());
child.WriteFloat(234, 3.14f).IgnoreError();
child.WritePackedFloat(234, GetFloatArray()).IgnoreError();
child.WriteRepeatedFloat(234, GetFloatVector()).IgnoreError();

child.WriteFloat(234, 3.14);
child.WritePackedDouble(234, GetDoubleArray());
child.WriteRepeatedDouble(234, GetDoubleVector());
child.WriteFloat(234, 3.14).IgnoreError();
child.WritePackedDouble(234, GetDoubleArray()).IgnoreError();
child.WriteRepeatedDouble(234, GetDoubleVector()).IgnoreError();

child.WriteBool(7, true);
child.WritePackedBool(8, GetBoolArray());
child.WriteRepeatedBool(8, GetBoolVector());
child.WriteBool(7, true).IgnoreError();
child.WritePackedBool(8, GetBoolArray()).IgnoreError();
child.WriteRepeatedBool(8, GetBoolVector()).IgnoreError();

encoder.WriteBytes(93, as_bytes(span<const double>(GetDoubleArray())));
encoder.WriteString(21343, kTestString);
encoder.WriteBytes(93, as_bytes(span<const double>(GetDoubleArray())))
.IgnoreError();
encoder.WriteString(21343, kTestString).IgnoreError();
}

stream::NullStream null_stream;
protobuf::StreamEncoder stream_encoder(null_stream, buffer);
stream_encoder.WriteBytesFromStream(3636, null_stream, 10824, buffer);
stream_encoder.WriteBytesFromStream(3636, null_stream, 10824, buffer)
.IgnoreError();
}

void CodeToPullInTableEncoder() {
Expand Down
4 changes: 2 additions & 2 deletions pw_router/size_report/base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ int main() {
PW_LOG_INFO("pw_StatusString %s", pw::OkStatus().str());

std::array<std::byte, sizeof(BasicPacket)> packet_buffer;
pw::sys_io::ReadBytes(packet_buffer);
pw::sys_io::WriteBytes(packet_buffer);
pw::sys_io::ReadBytes(packet_buffer).IgnoreError();
pw::sys_io::WriteBytes(packet_buffer).IgnoreError();

return static_cast<int>(packet.payload);
}
Loading

0 comments on commit 474eecb

Please sign in to comment.