Rename RETURN_IF_ERROR to RETURN_IF_NOT_OK (#31102)

* Add utils to create and prepare socket for tcp client

* Automated change: Fix sanity tests

* review comments

* review comments

* regenerate projects

* regenerate projects

* review comments

* comments

* fix typo

* remove unused parameter

* review comments

* remove static

* re-introduce forward declaration

* Rename RETURN_IF_ERROR to RETURN_IF_NOT_OK

Co-authored-by: Vignesh2208 <Vignesh2208@users.noreply.github.com>
pull/29235/head
Vignesh Babu 3 years ago committed by GitHub
parent 097729125a
commit 2e78e92dbe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 32
      src/core/ext/transport/binder/wire_format/wire_reader_impl.cc
  2. 12
      src/core/lib/event_engine/posix_engine/tcp_socket_utils.cc
  3. 2
      src/core/lib/gprpp/status_helper.h

@ -42,19 +42,19 @@ const char kAuthorityMetadataKey[] = ":authority";
absl::StatusOr<Metadata> parse_metadata(ReadableParcel* reader) { absl::StatusOr<Metadata> parse_metadata(ReadableParcel* reader) {
int num_header; int num_header;
RETURN_IF_ERROR(reader->ReadInt32(&num_header)); RETURN_IF_NOT_OK(reader->ReadInt32(&num_header));
if (num_header < 0) { if (num_header < 0) {
return absl::InvalidArgumentError("num_header cannot be negative"); return absl::InvalidArgumentError("num_header cannot be negative");
} }
std::vector<std::pair<std::string, std::string>> ret; std::vector<std::pair<std::string, std::string>> ret;
for (int i = 0; i < num_header; i++) { for (int i = 0; i < num_header; i++) {
int count; int count;
RETURN_IF_ERROR(reader->ReadInt32(&count)); RETURN_IF_NOT_OK(reader->ReadInt32(&count));
std::string key{}; std::string key{};
if (count > 0) RETURN_IF_ERROR(reader->ReadByteArray(&key)); if (count > 0) RETURN_IF_NOT_OK(reader->ReadByteArray(&key));
RETURN_IF_ERROR(reader->ReadInt32(&count)); RETURN_IF_NOT_OK(reader->ReadInt32(&count));
std::string value{}; std::string value{};
if (count > 0) RETURN_IF_ERROR(reader->ReadByteArray(&value)); if (count > 0) RETURN_IF_NOT_OK(reader->ReadByteArray(&value));
ret.emplace_back(key, value); ret.emplace_back(key, value);
} }
return ret; return ret;
@ -183,7 +183,7 @@ absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code,
} }
int version; int version;
RETURN_IF_ERROR(parcel->ReadInt32(&version)); RETURN_IF_NOT_OK(parcel->ReadInt32(&version));
gpr_log(GPR_DEBUG, "The other end respond with version = %d", version); gpr_log(GPR_DEBUG, "The other end respond with version = %d", version);
// We only support this single lowest possible version, so server must // We only support this single lowest possible version, so server must
// respond that version too. // respond that version too.
@ -194,7 +194,7 @@ absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code,
version, kWireFormatVersion); version, kWireFormatVersion);
} }
std::unique_ptr<Binder> binder{}; std::unique_ptr<Binder> binder{};
RETURN_IF_ERROR(parcel->ReadBinder(&binder)); RETURN_IF_NOT_OK(parcel->ReadBinder(&binder));
if (!binder) { if (!binder) {
return absl::InternalError("Read NULL binder from the parcel"); return absl::InternalError("Read NULL binder from the parcel");
} }
@ -210,7 +210,7 @@ absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code,
} }
case BinderTransportTxCode::ACKNOWLEDGE_BYTES: { case BinderTransportTxCode::ACKNOWLEDGE_BYTES: {
int64_t num_bytes = -1; int64_t num_bytes = -1;
RETURN_IF_ERROR(parcel->ReadInt64(&num_bytes)); RETURN_IF_NOT_OK(parcel->ReadInt64(&num_bytes));
gpr_log(GPR_DEBUG, "received acknowledge bytes = %" PRId64, num_bytes); gpr_log(GPR_DEBUG, "received acknowledge bytes = %" PRId64, num_bytes);
wire_writer_->OnAckReceived(num_bytes); wire_writer_->OnAckReceived(num_bytes);
break; break;
@ -220,14 +220,14 @@ absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code,
return absl::FailedPreconditionError("Receive PING request in client"); return absl::FailedPreconditionError("Receive PING request in client");
} }
int ping_id = -1; int ping_id = -1;
RETURN_IF_ERROR(parcel->ReadInt32(&ping_id)); RETURN_IF_NOT_OK(parcel->ReadInt32(&ping_id));
gpr_log(GPR_DEBUG, "received ping id = %d", ping_id); gpr_log(GPR_DEBUG, "received ping id = %d", ping_id);
// TODO(waynetu): Ping back. // TODO(waynetu): Ping back.
break; break;
} }
case BinderTransportTxCode::PING_RESPONSE: { case BinderTransportTxCode::PING_RESPONSE: {
int value = -1; int value = -1;
RETURN_IF_ERROR(parcel->ReadInt32(&value)); RETURN_IF_NOT_OK(parcel->ReadInt32(&value));
gpr_log(GPR_DEBUG, "received ping response = %d", value); gpr_log(GPR_DEBUG, "received ping response = %d", value);
break; break;
} }
@ -303,7 +303,7 @@ absl::Status WireReaderImpl::ProcessStreamingTransactionImpl(
gpr_log(GPR_INFO, "Total incoming bytes: %" PRId64, num_incoming_bytes_); gpr_log(GPR_INFO, "Total incoming bytes: %" PRId64, num_incoming_bytes_);
int flags; int flags;
RETURN_IF_ERROR(parcel->ReadInt32(&flags)); RETURN_IF_NOT_OK(parcel->ReadInt32(&flags));
*cancellation_flags = flags; *cancellation_flags = flags;
// Ignore in-coming transaction with flag = 0 to match with Java // Ignore in-coming transaction with flag = 0 to match with Java
@ -322,7 +322,7 @@ absl::Status WireReaderImpl::ProcessStreamingTransactionImpl(
gpr_log(GPR_DEBUG, "FLAG_MESSAGE_DATA = %d", (flags & kFlagMessageData)); gpr_log(GPR_DEBUG, "FLAG_MESSAGE_DATA = %d", (flags & kFlagMessageData));
gpr_log(GPR_DEBUG, "FLAG_SUFFIX = %d", (flags & kFlagSuffix)); gpr_log(GPR_DEBUG, "FLAG_SUFFIX = %d", (flags & kFlagSuffix));
int seq_num; int seq_num;
RETURN_IF_ERROR(parcel->ReadInt32(&seq_num)); RETURN_IF_NOT_OK(parcel->ReadInt32(&seq_num));
// TODO(waynetu): For now we'll just assume that the transactions commit in // TODO(waynetu): For now we'll just assume that the transactions commit in
// the same order they're issued. The following assertion detects // the same order they're issued. The following assertion detects
// out-of-order or missing transactions. WireReaderImpl should be fixed if // out-of-order or missing transactions. WireReaderImpl should be fixed if
@ -342,7 +342,7 @@ absl::Status WireReaderImpl::ProcessStreamingTransactionImpl(
if (flags & kFlagPrefix) { if (flags & kFlagPrefix) {
std::string method_ref; std::string method_ref;
if (!is_client_) { if (!is_client_) {
RETURN_IF_ERROR(parcel->ReadString(&method_ref)); RETURN_IF_NOT_OK(parcel->ReadString(&method_ref));
} }
absl::StatusOr<Metadata> initial_metadata_or_error = parse_metadata(parcel); absl::StatusOr<Metadata> initial_metadata_or_error = parse_metadata(parcel);
if (!initial_metadata_or_error.ok()) { if (!initial_metadata_or_error.ok()) {
@ -373,11 +373,11 @@ absl::Status WireReaderImpl::ProcessStreamingTransactionImpl(
} }
if (flags & kFlagMessageData) { if (flags & kFlagMessageData) {
int count; int count;
RETURN_IF_ERROR(parcel->ReadInt32(&count)); RETURN_IF_NOT_OK(parcel->ReadInt32(&count));
gpr_log(GPR_DEBUG, "count = %d", count); gpr_log(GPR_DEBUG, "count = %d", count);
std::string msg_data{}; std::string msg_data{};
if (count > 0) { if (count > 0) {
RETURN_IF_ERROR(parcel->ReadByteArray(&msg_data)); RETURN_IF_NOT_OK(parcel->ReadByteArray(&msg_data));
} }
message_buffer_[code] += msg_data; message_buffer_[code] += msg_data;
if ((flags & kFlagMessageDataIsPartial) == 0) { if ((flags & kFlagMessageDataIsPartial) == 0) {
@ -391,7 +391,7 @@ absl::Status WireReaderImpl::ProcessStreamingTransactionImpl(
if (flags & kFlagStatusDescription) { if (flags & kFlagStatusDescription) {
// FLAG_STATUS_DESCRIPTION set // FLAG_STATUS_DESCRIPTION set
std::string desc; std::string desc;
RETURN_IF_ERROR(parcel->ReadString(&desc)); RETURN_IF_NOT_OK(parcel->ReadString(&desc));
gpr_log(GPR_DEBUG, "description = %s", desc.c_str()); gpr_log(GPR_DEBUG, "description = %s", desc.c_str());
} }
Metadata trailing_metadata; Metadata trailing_metadata;

@ -113,17 +113,17 @@ absl::Status PrepareTcpClientSocket(PosixSocketWrapper sock,
close(sock.Fd()); close(sock.Fd());
} }
}); });
RETURN_IF_ERROR(sock.SetSocketNonBlocking(1)); RETURN_IF_NOT_OK(sock.SetSocketNonBlocking(1));
RETURN_IF_ERROR(sock.SetSocketCloexec(1)); RETURN_IF_NOT_OK(sock.SetSocketCloexec(1));
if (reinterpret_cast<const sockaddr*>(addr.address())->sa_family != AF_UNIX) { if (reinterpret_cast<const sockaddr*>(addr.address())->sa_family != AF_UNIX) {
// If its not a unix socket address. // If its not a unix socket address.
RETURN_IF_ERROR(sock.SetSocketLowLatency(1)); RETURN_IF_NOT_OK(sock.SetSocketLowLatency(1));
RETURN_IF_ERROR(sock.SetSocketReuseAddr(1)); RETURN_IF_NOT_OK(sock.SetSocketReuseAddr(1));
sock.TrySetSocketTcpUserTimeout(options, true); sock.TrySetSocketTcpUserTimeout(options, true);
} }
RETURN_IF_ERROR(sock.SetSocketNoSigpipeIfPossible()); RETURN_IF_NOT_OK(sock.SetSocketNoSigpipeIfPossible());
RETURN_IF_ERROR(sock.ApplySocketMutatorInOptions( RETURN_IF_NOT_OK(sock.ApplySocketMutatorInOptions(
GRPC_FD_CLIENT_CONNECTION_USAGE, options)); GRPC_FD_CLIENT_CONNECTION_USAGE, options));
// No errors. Set close_fd to false to ensure the socket is not closed. // No errors. Set close_fd to false to ensure the socket is not closed.
close_fd = false; close_fd = false;

@ -38,7 +38,7 @@ struct google_rpc_Status;
struct upb_Arena; struct upb_Arena;
} }
#define RETURN_IF_ERROR(expr) \ #define RETURN_IF_NOT_OK(expr) \
do { \ do { \
const absl::Status status = (expr); \ const absl::Status status = (expr); \
if (!status.ok()) return status; \ if (!status.ok()) return status; \

Loading…
Cancel
Save