From a135059ce7d2db3180045810f66dd69a8fc88284 Mon Sep 17 00:00:00 2001 From: kpayson64 Date: Fri, 13 Jul 2018 13:24:31 -0700 Subject: Make error strings consistent --- src/core/lib/iomgr/error.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/core/lib/iomgr/error.cc') diff --git a/src/core/lib/iomgr/error.cc b/src/core/lib/iomgr/error.cc index 90ed34da11..93a5c20abb 100644 --- a/src/core/lib/iomgr/error.cc +++ b/src/core/lib/iomgr/error.cc @@ -399,14 +399,14 @@ static grpc_error* copy_error_and_unref(grpc_error* in) { out = GRPC_ERROR_CREATE_FROM_STATIC_STRING("unknown"); if (in == GRPC_ERROR_NONE) { internal_set_str(&out, GRPC_ERROR_STR_DESCRIPTION, - grpc_slice_from_static_string("no error")); + grpc_slice_from_static_string("No Error")); internal_set_int(&out, GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_OK); } else if (in == GRPC_ERROR_OOM) { internal_set_str(&out, GRPC_ERROR_STR_DESCRIPTION, - grpc_slice_from_static_string("oom")); + grpc_slice_from_static_string("Out of memory")); } else if (in == GRPC_ERROR_CANCELLED) { internal_set_str(&out, GRPC_ERROR_STR_DESCRIPTION, - grpc_slice_from_static_string("cancelled")); + grpc_slice_from_static_string("Cancelled")); internal_set_int(&out, GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_CANCELLED); } } else if (gpr_ref_is_unique(&in->atomics.refs)) { -- cgit v1.2.3 From 1987e391bcdd572003fe9b6e1e27671203adebab Mon Sep 17 00:00:00 2001 From: kpayson64 Date: Wed, 25 Jul 2018 10:50:59 -0700 Subject: Revert "Make error strings consistent" This reverts commit a135059ce7d2db3180045810f66dd69a8fc88284. --- src/core/lib/iomgr/error.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/core/lib/iomgr/error.cc') diff --git a/src/core/lib/iomgr/error.cc b/src/core/lib/iomgr/error.cc index 93a5c20abb..90ed34da11 100644 --- a/src/core/lib/iomgr/error.cc +++ b/src/core/lib/iomgr/error.cc @@ -399,14 +399,14 @@ static grpc_error* copy_error_and_unref(grpc_error* in) { out = GRPC_ERROR_CREATE_FROM_STATIC_STRING("unknown"); if (in == GRPC_ERROR_NONE) { internal_set_str(&out, GRPC_ERROR_STR_DESCRIPTION, - grpc_slice_from_static_string("No Error")); + grpc_slice_from_static_string("no error")); internal_set_int(&out, GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_OK); } else if (in == GRPC_ERROR_OOM) { internal_set_str(&out, GRPC_ERROR_STR_DESCRIPTION, - grpc_slice_from_static_string("Out of memory")); + grpc_slice_from_static_string("oom")); } else if (in == GRPC_ERROR_CANCELLED) { internal_set_str(&out, GRPC_ERROR_STR_DESCRIPTION, - grpc_slice_from_static_string("Cancelled")); + grpc_slice_from_static_string("cancelled")); internal_set_int(&out, GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_CANCELLED); } } else if (gpr_ref_is_unique(&in->atomics.refs)) { -- cgit v1.2.3 From 1482d3f25c75d1773fd3ab7320403f93d173fc7f Mon Sep 17 00:00:00 2001 From: kpayson64 Date: Fri, 27 Jul 2018 12:21:05 -0700 Subject: PR Feedback Changes --- .../ext/filters/http/server/http_server_filter.cc | 15 ++++--------- .../filters/message_size/message_size_filter.cc | 12 +---------- src/core/lib/iomgr/error.cc | 14 ++++++++++++ src/core/lib/iomgr/error.h | 7 ++++++ .../lib/security/transport/server_auth_filter.cc | 25 +++++++++++++++++++++- src/core/lib/surface/call.cc | 2 +- src/core/lib/surface/server.cc | 23 ++++++++++++++++++-- 7 files changed, 72 insertions(+), 26 deletions(-) (limited to 'src/core/lib/iomgr/error.cc') diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index 01e5aa445e..c66f531a89 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -321,17 +321,8 @@ static void hs_recv_message_ready(void* user_data, grpc_error* err) { static void hs_recv_trailing_metadata_ready(void* user_data, grpc_error* err) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - if (calld->recv_initial_metadata_ready_error != GRPC_ERROR_NONE) { - if (err == GRPC_ERROR_NONE) { - err = GRPC_ERROR_REF(calld->recv_initial_metadata_ready_error); - } else if (err != calld->recv_initial_metadata_ready_error) { - err = grpc_error_add_child(err, calld->recv_initial_metadata_ready_error); - } else { - err = GRPC_ERROR_REF(err); - } - } else { - err = GRPC_ERROR_REF(err); - } + err = + grpc_error_maybe_add_child(err, calld->recv_initial_metadata_ready_error); GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); } @@ -382,6 +373,8 @@ static grpc_error* hs_mutate_op(grpc_call_element* elem, if (op->recv_trailing_metadata) { calld->original_recv_trailing_metadata_ready = op->payload->recv_trailing_metadata.recv_trailing_metadata_ready; + op->payload->recv_trailing_metadata.recv_trailing_metadata_ready = + &calld->recv_trailing_metadata_ready; } if (op->send_trailing_metadata) { diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index deb5ae70ec..b5ca1be804 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -155,17 +155,7 @@ static void recv_message_ready(void* user_data, grpc_error* error) { static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - if (calld->error != GRPC_ERROR_NONE) { - if (error == GRPC_ERROR_NONE) { - error = GRPC_ERROR_REF(calld->error); - } else if (error != calld->error) { - error = grpc_error_add_child(error, GRPC_ERROR_REF(calld->error)); - } else { - error = GRPC_ERROR_REF(error); - } - } else { - error = GRPC_ERROR_REF(error); - } + error = grpc_error_maybe_add_child(error, calld->error); // Invoke the next callback. GRPC_CLOSURE_RUN(calld->next_recv_trailing_metadata_ready, error); } diff --git a/src/core/lib/iomgr/error.cc b/src/core/lib/iomgr/error.cc index 90ed34da11..226b44c46d 100644 --- a/src/core/lib/iomgr/error.cc +++ b/src/core/lib/iomgr/error.cc @@ -518,6 +518,20 @@ grpc_error* grpc_error_add_child(grpc_error* src, grpc_error* child) { return new_err; } +grpc_error* grpc_error_maybe_add_child(grpc_error* src, grpc_error* child) { + if (src != GRPC_ERROR_NONE) { + if (child == GRPC_ERROR_NONE) { + return GRPC_ERROR_REF(src); + } else if (child != src) { + return grpc_error_add_child(src, GRPC_ERROR_REF(child)); + } else { + return GRPC_ERROR_REF(src); + } + } else { + return GRPC_ERROR_REF(child); + } +} + static const char* no_error_string = "\"No Error\""; static const char* oom_error_string = "\"Out of memory\""; static const char* cancelled_error_string = "\"Cancelled\""; diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index 27c4d22fd1..4f65c447fd 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -187,6 +187,13 @@ bool grpc_error_get_str(grpc_error* error, grpc_error_strs which, /// child error. grpc_error* grpc_error_add_child(grpc_error* src, grpc_error* child) GRPC_MUST_USE_RESULT; +/// Produce an error that is a combination of both src and child. +// If src or child, is GRPC_ERROR_NONE, a new reference to the other error is +// returned. Otherwise, a new error with src as the parent and child as the +// child is returned. +grpc_error* grpc_error_maybe_add_child(grpc_error* src, + grpc_error* child) GRPC_MUST_USE_RESULT; + grpc_error* grpc_os_error(const char* file, int line, int err, const char* call_name) GRPC_MUST_USE_RESULT; diff --git a/src/core/lib/security/transport/server_auth_filter.cc b/src/core/lib/security/transport/server_auth_filter.cc index 2dbefdf131..7732b695b3 100644 --- a/src/core/lib/security/transport/server_auth_filter.cc +++ b/src/core/lib/security/transport/server_auth_filter.cc @@ -41,6 +41,9 @@ struct call_data { grpc_transport_stream_op_batch* recv_initial_metadata_batch; grpc_closure* original_recv_initial_metadata_ready; grpc_closure recv_initial_metadata_ready; + grpc_error* error; + grpc_closure recv_trailing_metadata_ready; + grpc_closure* original_recv_trailing_metadata_ready; grpc_metadata_array md; const grpc_metadata* consumed_md; size_t num_consumed_md; @@ -111,6 +114,7 @@ static void on_md_processing_done_inner(grpc_call_element* elem, batch->payload->recv_initial_metadata.recv_initial_metadata, remove_consumed_md, elem, "Response metadata filtering error"); } + calld->error = GRPC_ERROR_REF(error); GRPC_CLOSURE_SCHED(calld->original_recv_initial_metadata_ready, error); } @@ -186,6 +190,13 @@ static void recv_initial_metadata_ready(void* arg, grpc_error* error) { GRPC_ERROR_REF(error)); } +static void recv_trailing_metadata_ready(void* user_data, grpc_error* err) { + grpc_call_element* elem = static_cast(user_data); + call_data* calld = static_cast(elem->call_data); + err = grpc_error_maybe_add_child(err, calld->error); + GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); +} + static void auth_start_transport_stream_op_batch( grpc_call_element* elem, grpc_transport_stream_op_batch* batch) { call_data* calld = static_cast(elem->call_data); @@ -197,6 +208,12 @@ static void auth_start_transport_stream_op_batch( batch->payload->recv_initial_metadata.recv_initial_metadata_ready = &calld->recv_initial_metadata_ready; } + if (batch->recv_trailing_metadata) { + calld->original_recv_trailing_metadata_ready = + batch->payload->recv_trailing_metadata.recv_trailing_metadata_ready; + batch->payload->recv_trailing_metadata.recv_trailing_metadata_ready = + &calld->recv_trailing_metadata_ready; + } grpc_call_next_op(elem, batch); } @@ -210,6 +227,9 @@ static grpc_error* init_call_elem(grpc_call_element* elem, GRPC_CLOSURE_INIT(&calld->recv_initial_metadata_ready, recv_initial_metadata_ready, elem, grpc_schedule_on_exec_ctx); + GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, + recv_trailing_metadata_ready, elem, + grpc_schedule_on_exec_ctx); // Create server security context. Set its auth context from channel // data and save it in the call context. grpc_server_security_context* server_ctx = @@ -229,7 +249,10 @@ static grpc_error* init_call_elem(grpc_call_element* elem, /* Destructor for call_data */ static void destroy_call_elem(grpc_call_element* elem, const grpc_call_final_info* final_info, - grpc_closure* ignored) {} + grpc_closure* ignored) { + call_data* calld = static_cast(elem->call_data); + GRPC_ERROR_UNREF(calld->error); +} /* Constructor for channel_data */ static grpc_error* init_channel_elem(grpc_channel_element* elem, diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 1ee3667a58..151cf6c852 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -512,7 +512,7 @@ static void destroy_call(void* call, grpc_error* error) { grpc_slice slice = grpc_empty_slice(); grpc_error_get_status(c->status_error, c->send_deadline, &c->final_info.final_status, &slice, nullptr, - &c->final_info.error_string); + &(c->final_info.error_string)); GRPC_ERROR_UNREF(c->status_error); c->final_info.stats.latency = gpr_time_sub(gpr_now(GPR_CLOCK_MONOTONIC), c->start_time); diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index cb34def740..4403caf044 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -149,6 +149,9 @@ struct call_data { grpc_closure server_on_recv_initial_metadata; grpc_closure kill_zombie_closure; grpc_closure* on_done_recv_initial_metadata; + grpc_closure recv_trailing_metadata_ready; + grpc_error* error; + grpc_closure* original_recv_trailing_metadata_ready; grpc_closure publish; @@ -730,6 +733,14 @@ static void server_on_recv_initial_metadata(void* ptr, grpc_error* error) { GRPC_CLOSURE_RUN(calld->on_done_recv_initial_metadata, error); } +static void server_recv_trailing_metadata_ready(void* user_data, + grpc_error* err) { + grpc_call_element* elem = static_cast(user_data); + call_data* calld = static_cast(elem->call_data); + err = grpc_error_maybe_add_child(err, calld->error); + GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); +} + static void server_mutate_op(grpc_call_element* elem, grpc_transport_stream_op_batch* op) { call_data* calld = static_cast(elem->call_data); @@ -745,6 +756,12 @@ static void server_mutate_op(grpc_call_element* elem, op->payload->recv_initial_metadata.recv_flags = &calld->recv_initial_metadata_flags; } + if (op->recv_trailing_metadata) { + calld->original_recv_trailing_metadata_ready = + op->payload->recv_trailing_metadata.recv_trailing_metadata_ready; + op->payload->recv_trailing_metadata.recv_trailing_metadata_ready = + &calld->recv_trailing_metadata_ready; + } } static void server_start_transport_stream_op_batch( @@ -828,7 +845,9 @@ static grpc_error* init_call_elem(grpc_call_element* elem, GRPC_CLOSURE_INIT(&calld->server_on_recv_initial_metadata, server_on_recv_initial_metadata, elem, grpc_schedule_on_exec_ctx); - + GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, + server_recv_trailing_metadata_ready, elem, + grpc_schedule_on_exec_ctx); server_ref(chand->server); return GRPC_ERROR_NONE; } @@ -840,7 +859,7 @@ static void destroy_call_elem(grpc_call_element* elem, call_data* calld = static_cast(elem->call_data); GPR_ASSERT(calld->state != PENDING); - + GRPC_ERROR_UNREF(calld->error); if (calld->host_set) { grpc_slice_unref_internal(calld->host); } -- cgit v1.2.3 From 85ff14dd16524b408bd30eb76d790be126aebb90 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 21 Aug 2018 10:11:53 -0700 Subject: Modify existing error child manipulation --- src/core/ext/filters/http/client/http_client_filter.cc | 9 ++------- src/core/ext/filters/http/server/http_server_filter.cc | 5 +++-- src/core/ext/filters/message_size/message_size_filter.cc | 3 ++- src/core/lib/iomgr/error.cc | 16 ++++++---------- src/core/lib/iomgr/error.h | 6 ------ src/core/lib/security/transport/server_auth_filter.cc | 2 +- src/core/lib/surface/call.cc | 4 +--- src/core/lib/surface/server.cc | 2 +- 8 files changed, 16 insertions(+), 31 deletions(-) (limited to 'src/core/lib/iomgr/error.cc') diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index 04ac4ac947..f44dc032a7 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -164,13 +164,8 @@ static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { } else { GRPC_ERROR_REF(error); } - if (calld->recv_initial_metadata_error != GRPC_ERROR_NONE) { - if (error == GRPC_ERROR_NONE) { - error = GRPC_ERROR_REF(calld->recv_initial_metadata_error); - } else if (error != calld->recv_initial_metadata_error) { - error = grpc_error_add_child(error, calld->recv_initial_metadata_error); - } - } + error = grpc_error_add_child( + error, GRPC_ERROR_REF(calld->recv_initial_metadata_error)); GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, error); } diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index c66f531a89..926afeec84 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -321,8 +321,9 @@ static void hs_recv_message_ready(void* user_data, grpc_error* err) { static void hs_recv_trailing_metadata_ready(void* user_data, grpc_error* err) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - err = - grpc_error_maybe_add_child(err, calld->recv_initial_metadata_ready_error); + err = grpc_error_add_child( + GRPC_ERROR_REF(err), + GRPC_ERROR_REF(calld->recv_initial_metadata_ready_error)); GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); } diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index b5ca1be804..a5f5f8e2ff 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -155,7 +155,8 @@ static void recv_message_ready(void* user_data, grpc_error* error) { static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - error = grpc_error_maybe_add_child(error, calld->error); + error = + grpc_error_add_child(GRPC_ERROR_REF(error), GRPC_ERROR_REF(calld->error)); // Invoke the next callback. GRPC_CLOSURE_RUN(calld->next_recv_trailing_metadata_ready, error); } diff --git a/src/core/lib/iomgr/error.cc b/src/core/lib/iomgr/error.cc index 226b44c46d..dfd063a695 100644 --- a/src/core/lib/iomgr/error.cc +++ b/src/core/lib/iomgr/error.cc @@ -513,22 +513,18 @@ bool grpc_error_get_str(grpc_error* err, grpc_error_strs which, grpc_error* grpc_error_add_child(grpc_error* src, grpc_error* child) { GPR_TIMER_SCOPE("grpc_error_add_child", 0); - grpc_error* new_err = copy_error_and_unref(src); - internal_add_error(&new_err, child); - return new_err; -} - -grpc_error* grpc_error_maybe_add_child(grpc_error* src, grpc_error* child) { if (src != GRPC_ERROR_NONE) { if (child == GRPC_ERROR_NONE) { - return GRPC_ERROR_REF(src); + return src; } else if (child != src) { - return grpc_error_add_child(src, GRPC_ERROR_REF(child)); + grpc_error* new_err = copy_error_and_unref(src); + internal_add_error(&new_err, child); + return new_err; } else { - return GRPC_ERROR_REF(src); + return src; } } else { - return GRPC_ERROR_REF(child); + return child; } } diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index 4f65c447fd..e5369695c9 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -187,12 +187,6 @@ bool grpc_error_get_str(grpc_error* error, grpc_error_strs which, /// child error. grpc_error* grpc_error_add_child(grpc_error* src, grpc_error* child) GRPC_MUST_USE_RESULT; -/// Produce an error that is a combination of both src and child. -// If src or child, is GRPC_ERROR_NONE, a new reference to the other error is -// returned. Otherwise, a new error with src as the parent and child as the -// child is returned. -grpc_error* grpc_error_maybe_add_child(grpc_error* src, - grpc_error* child) GRPC_MUST_USE_RESULT; grpc_error* grpc_os_error(const char* file, int line, int err, const char* call_name) GRPC_MUST_USE_RESULT; diff --git a/src/core/lib/security/transport/server_auth_filter.cc b/src/core/lib/security/transport/server_auth_filter.cc index 5f2ad261d2..552e70130a 100644 --- a/src/core/lib/security/transport/server_auth_filter.cc +++ b/src/core/lib/security/transport/server_auth_filter.cc @@ -191,7 +191,7 @@ static void recv_initial_metadata_ready(void* arg, grpc_error* error) { static void recv_trailing_metadata_ready(void* user_data, grpc_error* err) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - err = grpc_error_maybe_add_child(err, calld->error); + err = grpc_error_add_child(GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->error)); GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); } diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 56c4562952..d2c14571d3 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -686,6 +686,7 @@ static void cancel_with_status(grpc_call* c, grpc_status_code status, } static void set_final_status(grpc_call* call, grpc_error* error) { + gpr_log(GPR_INFO, "set final status"); if (grpc_call_error_trace.enabled()) { gpr_log(GPR_DEBUG, "set_final_status %s", call->is_client ? "CLI" : "SVR"); gpr_log(GPR_DEBUG, "%s", grpc_error_string(error)); @@ -1650,9 +1651,6 @@ static grpc_call_error call_start_batch(grpc_call* call, const grpc_op* ops, grpc_slice_ref_internal( *op->data.send_status_from_server.status_details)); call->send_extra_metadata_count++; - char* msg = grpc_slice_to_c_string( - GRPC_MDVALUE(call->send_extra_metadata[1].md)); - gpr_free(msg); } grpc_error* status_error = op->data.send_status_from_server.status == GRPC_STATUS_OK diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index 4403caf044..521825ca69 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -737,7 +737,7 @@ static void server_recv_trailing_metadata_ready(void* user_data, grpc_error* err) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - err = grpc_error_maybe_add_child(err, calld->error); + err = grpc_error_add_child(GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->error)); GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); } -- cgit v1.2.3 From 1cfd81a604699d6ab8095c90e7da5fc588bb3048 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 21 Aug 2018 16:25:40 -0700 Subject: Explain the newer semantics of grpc_error_add_child --- src/core/lib/iomgr/error.cc | 5 +++++ src/core/lib/iomgr/error.h | 7 +++++++ test/core/iomgr/error_test.cc | 11 ----------- 3 files changed, 12 insertions(+), 11 deletions(-) (limited to 'src/core/lib/iomgr/error.cc') diff --git a/src/core/lib/iomgr/error.cc b/src/core/lib/iomgr/error.cc index dfd063a695..13bc69ffb6 100644 --- a/src/core/lib/iomgr/error.cc +++ b/src/core/lib/iomgr/error.cc @@ -515,15 +515,20 @@ grpc_error* grpc_error_add_child(grpc_error* src, grpc_error* child) { GPR_TIMER_SCOPE("grpc_error_add_child", 0); if (src != GRPC_ERROR_NONE) { if (child == GRPC_ERROR_NONE) { + /* \a child is empty. Simply return the ref to \a src */ return src; } else if (child != src) { grpc_error* new_err = copy_error_and_unref(src); internal_add_error(&new_err, child); return new_err; } else { + /* \a src and \a child are the same. Drop one of the references and return + * the other */ + GRPC_ERROR_UNREF(child); return src; } } else { + /* \a src is empty. Simply return the ref to \a child */ return child; } } diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index e5369695c9..49f4029bc2 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -185,6 +185,13 @@ bool grpc_error_get_str(grpc_error* error, grpc_error_strs which, /// error occurring. Allows root causing high level errors from lower level /// errors that contributed to them. The src error takes ownership of the /// child error. +/// +/// Edge Conditions - +/// 1) If either of \a src or \a child is GRPC_ERROR_NONE, returns a reference +/// to the other argument. 2) If both \a src and \a child are GRPC_ERROR_NONE, +/// returns GRPC_ERROR_NONE. 3) If \a src and \a child point to the same error, +/// returns a single reference. (Note that, 2 references should have been +/// received to the error in this case.) grpc_error* grpc_error_add_child(grpc_error* src, grpc_error* child) GRPC_MUST_USE_RESULT; diff --git a/test/core/iomgr/error_test.cc b/test/core/iomgr/error_test.cc index a1628a1f71..d78a8c2af3 100644 --- a/test/core/iomgr/error_test.cc +++ b/test/core/iomgr/error_test.cc @@ -187,16 +187,6 @@ static void test_os_error() { GRPC_ERROR_UNREF(error); } -static void test_special() { - grpc_error* error = GRPC_ERROR_NONE; - error = grpc_error_add_child( - error, GRPC_ERROR_CREATE_FROM_STATIC_STRING("test child")); - intptr_t i; - GPR_ASSERT(grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &i)); - GPR_ASSERT(i == GRPC_STATUS_OK); - GRPC_ERROR_UNREF(error); -} - static void test_overflow() { grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Overflow"); @@ -235,7 +225,6 @@ int main(int argc, char** argv) { test_os_error(); test_create_referencing(); test_create_referencing_many(); - test_special(); test_overflow(); grpc_shutdown(); -- cgit v1.2.3