pw_status: Remove implicit conversion to pw_Status
Conversions to the pw_Status enum are dangerous, since they make Status
convertible to bool. This means statements like `if (status)` compile,
and status evaluates false for OK and true for non-OK.
This change replaces pw::Status's conversion to pw_Status with an
explicit code() function.
Change-Id: I2075a73c3e59ec051bd50b60a089baefc5f05dca
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/24520
Reviewed-by: Ewout van Bekkum <ewout@google.com>
Commit-Queue: Wyatt Hepler <hepler@google.com>
diff --git a/pw_kvs/key_value_store_binary_format_test.cc b/pw_kvs/key_value_store_binary_format_test.cc
index ea1b596..76c0667 100644
--- a/pw_kvs/key_value_store_binary_format_test.cc
+++ b/pw_kvs/key_value_store_binary_format_test.cc
@@ -725,7 +725,7 @@
EXPECT_EQ(false, kvs_.error_detected());
- EXPECT_EQ(false, kvs_.Init());
+ EXPECT_EQ(Status::Ok(), kvs_.Init());
stats = kvs_.GetStorageStats();
EXPECT_EQ(stats.in_use_bytes, (192u * kvs_.redundancy()));
EXPECT_EQ(stats.reclaimable_bytes, 0u);
diff --git a/pw_ring_buffer/prefixed_entry_ring_buffer_test.cc b/pw_ring_buffer/prefixed_entry_ring_buffer_test.cc
index 5838ca4..6469f42 100644
--- a/pw_ring_buffer/prefixed_entry_ring_buffer_test.cc
+++ b/pw_ring_buffer/prefixed_entry_ring_buffer_test.cc
@@ -397,7 +397,7 @@
T item;
} aliased;
size_t bytes_read = 0;
- PW_CHECK_INT_EQ(ring.PeekFront(aliased.buffer, &bytes_read), Status::Ok());
+ PW_CHECK_OK(ring.PeekFront(aliased.buffer, &bytes_read));
PW_CHECK_INT_EQ(bytes_read, sizeof(T));
return aliased.item;
}
diff --git a/pw_rpc/nanopb/codegen_test.cc b/pw_rpc/nanopb/codegen_test.cc
index 2ca01ab..7c7667e 100644
--- a/pw_rpc/nanopb/codegen_test.cc
+++ b/pw_rpc/nanopb/codegen_test.cc
@@ -56,27 +56,27 @@
PW_NANOPB_TEST_METHOD_CONTEXT(test::TestService, TestRpc) context;
EXPECT_EQ(Status::Ok(),
- context.call({.integer = 123, .status_code = Status::Ok()}));
+ context.call({.integer = 123, .status_code = Status::Ok().code()}));
EXPECT_EQ(124, context.response().value);
- EXPECT_EQ(
- Status::InvalidArgument(),
- context.call({.integer = 999, .status_code = Status::InvalidArgument()}));
+ EXPECT_EQ(Status::InvalidArgument(),
+ context.call({.integer = 999,
+ .status_code = Status::InvalidArgument().code()}));
EXPECT_EQ(1000, context.response().value);
}
TEST(NanopbCodegen, Server_InvokeStreamingRpc) {
PW_NANOPB_TEST_METHOD_CONTEXT(test::TestService, TestStreamRpc) context;
- context.call({.integer = 0, .status_code = Status::Aborted()});
+ context.call({.integer = 0, .status_code = Status::Aborted().code()});
EXPECT_EQ(Status::Aborted(), context.status());
EXPECT_TRUE(context.done());
EXPECT_TRUE(context.responses().empty());
EXPECT_EQ(0u, context.total_responses());
- context.call({.integer = 4, .status_code = Status::Ok()});
+ context.call({.integer = 4, .status_code = Status::Ok().code()});
ASSERT_EQ(4u, context.responses().size());
ASSERT_EQ(4u, context.total_responses());
@@ -85,7 +85,7 @@
EXPECT_EQ(context.responses()[i].number, i);
}
- EXPECT_EQ(Status::Ok(), context.status());
+ EXPECT_EQ(Status::Ok().code(), context.status());
}
TEST(NanopbCodegen,
@@ -94,7 +94,7 @@
ASSERT_EQ(3u, context.responses().max_size());
- context.call({.integer = 5, .status_code = Status::NotFound()});
+ context.call({.integer = 5, .status_code = Status::NotFound().code()});
ASSERT_EQ(3u, context.responses().size());
ASSERT_EQ(5u, context.total_responses());
diff --git a/pw_rpc/packet.cc b/pw_rpc/packet.cc
index e9270cd..9b62a4f 100644
--- a/pw_rpc/packet.cc
+++ b/pw_rpc/packet.cc
@@ -80,7 +80,7 @@
rpc_packet.WriteChannelId(channel_id_);
rpc_packet.WriteServiceId(service_id_);
rpc_packet.WriteMethodId(method_id_);
- rpc_packet.WriteStatus(status_);
+ rpc_packet.WriteStatus(status_.code());
return encoder.Encode();
}
diff --git a/pw_rpc/raw/codegen_test.cc b/pw_rpc/raw/codegen_test.cc
index 7041cc1..6194031 100644
--- a/pw_rpc/raw/codegen_test.cc
+++ b/pw_rpc/raw/codegen_test.cc
@@ -93,7 +93,7 @@
protobuf::NestedEncoder encoder(buffer);
test::TestRequest::Encoder test_request(&encoder);
test_request.WriteInteger(123);
- test_request.WriteStatusCode(Status::Ok());
+ test_request.WriteStatusCode(Status::Ok().code());
auto sws = context.call(encoder.Encode().value());
EXPECT_EQ(Status::Ok(), sws.status());
@@ -119,7 +119,7 @@
protobuf::NestedEncoder encoder(buffer);
test::TestRequest::Encoder test_request(&encoder);
test_request.WriteInteger(5);
- test_request.WriteStatusCode(Status::Unauthenticated());
+ test_request.WriteStatusCode(Status::Unauthenticated().code());
context.call(encoder.Encode().value());
EXPECT_TRUE(context.done());
diff --git a/pw_status/public/pw_status/status.h b/pw_status/public/pw_status/status.h
index bf6c6d8..2842797 100644
--- a/pw_status/public/pw_status/status.h
+++ b/pw_status/public/pw_status/status.h
@@ -307,9 +307,6 @@
constexpr Status(const Status&) = default;
constexpr Status& operator=(const Status&) = default;
- // Status implicitly converts to a Status::Code.
- constexpr operator Code() const { return code_; }
-
// Returns the Status::Code (pw_Status) for this Status.
constexpr Code code() const { return code_; }
@@ -373,6 +370,20 @@
Code code_;
};
+constexpr bool operator==(const Status& lhs, const Status& rhs) {
+ return lhs.code() == rhs.code();
+}
+
+constexpr bool operator!=(const Status& lhs, const Status& rhs) {
+ return lhs.code() != rhs.code();
+}
+
} // namespace pw
+// Create a C++ overload of pw_StatusString so that it supports pw::Status in
+// addition to pw_Status.
+inline const char* pw_StatusString(pw::Status status) {
+ return pw_StatusString(status.code());
+}
+
#endif // __cplusplus
diff --git a/pw_status/public/pw_status/status_with_size.h b/pw_status/public/pw_status/status_with_size.h
index d83548e..751ace8 100644
--- a/pw_status/public/pw_status/status_with_size.h
+++ b/pw_status/public/pw_status/status_with_size.h
@@ -30,8 +30,8 @@
private:
friend class ::pw::StatusWithSize;
- explicit constexpr StatusWithSizeConstant(Status::Code value)
- : value_(static_cast<size_t>(value) << kStatusShift) {}
+ explicit constexpr StatusWithSizeConstant(Status value)
+ : value_(static_cast<size_t>(value.code()) << kStatusShift) {}
const size_t value_;
};
@@ -163,7 +163,8 @@
// Creates a StatusWithSize with the provided status and size.
explicit constexpr StatusWithSize(Status status, size_t size)
- : StatusWithSize((static_cast<size_t>(status) << kStatusShift) | size) {}
+ : StatusWithSize((static_cast<size_t>(status.code()) << kStatusShift) |
+ size) {}
// Allow implicit conversions from the StatusWithSize constants.
constexpr StatusWithSize(Constant constant) : size_(constant.value_) {}
diff --git a/pw_status/status_test.cc b/pw_status/status_test.cc
index 47e4fb7..4812b9d 100644
--- a/pw_status/status_test.cc
+++ b/pw_status/status_test.cc
@@ -202,7 +202,7 @@
Status::Code PassStatusFromC(Status status);
-Status::Code PassStatusFromCpp(Status status) { return status; }
+Status::Code PassStatusFromCpp(Status status) { return status.code(); }
int TestStatusFromC(void);
diff --git a/pw_status/status_with_size_test.cc b/pw_status/status_with_size_test.cc
index e856713..ef0f1cf 100644
--- a/pw_status/status_with_size_test.cc
+++ b/pw_status/status_with_size_test.cc
@@ -67,7 +67,7 @@
for (int i = 0; i < 32; ++i) {
StatusWithSize result(static_cast<Status::Code>(i), 0);
EXPECT_EQ(result.ok(), i == 0);
- EXPECT_EQ(i, static_cast<int>(result.status()));
+ EXPECT_EQ(i, static_cast<int>(result.status().code()));
EXPECT_EQ(0u, result.size());
}
}
@@ -76,7 +76,7 @@
for (int i = 0; i < 32; ++i) {
StatusWithSize result(static_cast<Status::Code>(i), i);
EXPECT_EQ(result.ok(), i == 0);
- EXPECT_EQ(i, static_cast<int>(result.status()));
+ EXPECT_EQ(i, static_cast<int>(result.status().code()));
EXPECT_EQ(static_cast<size_t>(i), result.size());
}
}
@@ -86,7 +86,7 @@
StatusWithSize result(static_cast<Status::Code>(i),
StatusWithSize::max_size());
EXPECT_EQ(result.ok(), i == 0);
- EXPECT_EQ(i, static_cast<int>(result.status()));
+ EXPECT_EQ(i, static_cast<int>(result.status().code()));
EXPECT_EQ(result.max_size(), result.size());
}
}
diff --git a/pw_trace_tokenized/trace.cc b/pw_trace_tokenized/trace.cc
index b017a73..3769ca2 100644
--- a/pw_trace_tokenized/trace.cc
+++ b/pw_trace_tokenized/trace.cc
@@ -50,7 +50,7 @@
flags,
data_buffer,
data_size)
- .Ok()) {
+ .ok()) {
// Queue full dropping sample
// TODO(rgoliver): Allow other strategies, for example: drop oldest, try
// empty queue, or block.
@@ -318,12 +318,14 @@
pw_trace_SinkEndBlock end_block_func,
void* user_data,
pw_trace_SinkHandle* handle) {
- return Callbacks::Instance().RegisterSink(
- start_func, add_bytes_func, end_block_func, user_data, handle);
+ return Callbacks::Instance()
+ .RegisterSink(
+ start_func, add_bytes_func, end_block_func, user_data, handle)
+ .code();
}
pw_Status pw_trace_UnregisterSink(pw_trace_EventCallbackHandle handle) {
- return Callbacks::Instance().UnregisterSink(handle);
+ return Callbacks::Instance().UnregisterSink(handle).code();
}
pw_Status pw_trace_RegisterEventCallback(
@@ -331,16 +333,18 @@
pw_trace_ShouldCallOnEveryEvent called_on_every_event,
void* user_data,
pw_trace_EventCallbackHandle* handle) {
- return Callbacks::Instance().RegisterEventCallback(
- callback,
- static_cast<CallbacksImpl::CallOnEveryEvent>(called_on_every_event),
- user_data,
- handle);
+ return Callbacks::Instance()
+ .RegisterEventCallback(
+ callback,
+ static_cast<CallbacksImpl::CallOnEveryEvent>(called_on_every_event),
+ user_data,
+ handle)
+ .code();
}
pw_Status pw_trace_UnregisterEventCallback(
pw_trace_EventCallbackHandle handle) {
- return Callbacks::Instance().UnregisterEventCallback(handle);
+ return Callbacks::Instance().UnregisterEventCallback(handle).code();
}
PW_EXTERN_C_END
