Address initial comments

pull/18807/head
Arjun Roy 6 years ago
parent 4e24b0e4dc
commit e9a79f928c
  1. 6
      src/core/ext/filters/http/client/http_client_filter.cc
  2. 37
      src/core/ext/filters/http/server/http_server_filter.cc
  3. 4
      src/core/ext/transport/chttp2/transport/chttp2_transport.cc
  4. 19
      src/core/lib/transport/metadata.h
  5. 6
      src/core/lib/transport/status_metadata.cc

@ -107,8 +107,8 @@ static grpc_error* client_filter_incoming_metadata(grpc_call_element* elem,
* https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md. * https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md.
*/ */
if (b->idx.named.grpc_status != nullptr || if (b->idx.named.grpc_status != nullptr ||
grpc_mdelem_eq_static(b->idx.named.status->md, grpc_mdelem_static_value_eq(b->idx.named.status->md,
GRPC_MDELEM_STATUS_200)) { GRPC_MDELEM_STATUS_200)) {
grpc_metadata_batch_remove(b, b->idx.named.status); grpc_metadata_batch_remove(b, b->idx.named.status);
} else { } else {
char* val = grpc_dump_slice(GRPC_MDVALUE(b->idx.named.status->md), char* val = grpc_dump_slice(GRPC_MDVALUE(b->idx.named.status->md),
@ -141,7 +141,7 @@ static grpc_error* client_filter_incoming_metadata(grpc_call_element* elem,
} }
if (b->idx.named.content_type != nullptr) { if (b->idx.named.content_type != nullptr) {
if (!grpc_mdelem_eq_static( if (!grpc_mdelem_static_value_eq(
b->idx.named.content_type->md, b->idx.named.content_type->md,
GRPC_MDELEM_CONTENT_TYPE_APPLICATION_SLASH_GRPC)) { GRPC_MDELEM_CONTENT_TYPE_APPLICATION_SLASH_GRPC)) {
if (grpc_slice_buf_start_eq(GRPC_MDVALUE(b->idx.named.content_type->md), if (grpc_slice_buf_start_eq(GRPC_MDVALUE(b->idx.named.content_type->md),

@ -131,23 +131,23 @@ static grpc_error* hs_filter_incoming_metadata(grpc_call_element* elem,
static const char* error_name = "Failed processing incoming headers"; static const char* error_name = "Failed processing incoming headers";
if (b->idx.named.method != nullptr) { if (b->idx.named.method != nullptr) {
if (grpc_mdelem_eq_static(b->idx.named.method->md, if (grpc_mdelem_static_value_eq(b->idx.named.method->md,
GRPC_MDELEM_METHOD_GET)) { GRPC_MDELEM_METHOD_POST)) {
*calld->recv_initial_metadata_flags |=
GRPC_INITIAL_METADATA_CACHEABLE_REQUEST;
*calld->recv_initial_metadata_flags &=
~GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST;
} else if (grpc_mdelem_eq_static(b->idx.named.method->md,
GRPC_MDELEM_METHOD_POST)) {
*calld->recv_initial_metadata_flags &= *calld->recv_initial_metadata_flags &=
~(GRPC_INITIAL_METADATA_CACHEABLE_REQUEST | ~(GRPC_INITIAL_METADATA_CACHEABLE_REQUEST |
GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST); GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST);
} else if (grpc_mdelem_eq_static(b->idx.named.method->md, } else if (grpc_mdelem_static_value_eq(b->idx.named.method->md,
GRPC_MDELEM_METHOD_PUT)) { GRPC_MDELEM_METHOD_PUT)) {
*calld->recv_initial_metadata_flags &= *calld->recv_initial_metadata_flags &=
~GRPC_INITIAL_METADATA_CACHEABLE_REQUEST; ~GRPC_INITIAL_METADATA_CACHEABLE_REQUEST;
*calld->recv_initial_metadata_flags |= *calld->recv_initial_metadata_flags |=
GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST; GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST;
} else if (grpc_mdelem_static_value_eq(b->idx.named.method->md,
GRPC_MDELEM_METHOD_GET)) {
*calld->recv_initial_metadata_flags |=
GRPC_INITIAL_METADATA_CACHEABLE_REQUEST;
*calld->recv_initial_metadata_flags &=
~GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST;
} else { } else {
hs_add_error(error_name, &error, hs_add_error(error_name, &error,
grpc_attach_md_to_error( grpc_attach_md_to_error(
@ -164,7 +164,8 @@ static grpc_error* hs_filter_incoming_metadata(grpc_call_element* elem,
} }
if (b->idx.named.te != nullptr) { if (b->idx.named.te != nullptr) {
if (!grpc_mdelem_eq_static(b->idx.named.te->md, GRPC_MDELEM_TE_TRAILERS)) { if (!grpc_mdelem_static_value_eq(b->idx.named.te->md,
GRPC_MDELEM_TE_TRAILERS)) {
hs_add_error(error_name, &error, hs_add_error(error_name, &error,
grpc_attach_md_to_error( grpc_attach_md_to_error(
GRPC_ERROR_CREATE_FROM_STATIC_STRING("Bad header"), GRPC_ERROR_CREATE_FROM_STATIC_STRING("Bad header"),
@ -179,12 +180,12 @@ static grpc_error* hs_filter_incoming_metadata(grpc_call_element* elem,
} }
if (b->idx.named.scheme != nullptr) { if (b->idx.named.scheme != nullptr) {
if (!grpc_mdelem_eq_static(b->idx.named.scheme->md, if (!grpc_mdelem_static_value_eq(b->idx.named.scheme->md,
GRPC_MDELEM_SCHEME_HTTP) && GRPC_MDELEM_SCHEME_HTTP) &&
!grpc_mdelem_eq_static(b->idx.named.scheme->md, !grpc_mdelem_static_value_eq(b->idx.named.scheme->md,
GRPC_MDELEM_SCHEME_HTTPS) && GRPC_MDELEM_SCHEME_HTTPS) &&
!grpc_mdelem_eq_static(b->idx.named.scheme->md, !grpc_mdelem_static_value_eq(b->idx.named.scheme->md,
GRPC_MDELEM_SCHEME_GRPC)) { GRPC_MDELEM_SCHEME_GRPC)) {
hs_add_error(error_name, &error, hs_add_error(error_name, &error,
grpc_attach_md_to_error( grpc_attach_md_to_error(
GRPC_ERROR_CREATE_FROM_STATIC_STRING("Bad header"), GRPC_ERROR_CREATE_FROM_STATIC_STRING("Bad header"),
@ -200,7 +201,7 @@ static grpc_error* hs_filter_incoming_metadata(grpc_call_element* elem,
} }
if (b->idx.named.content_type != nullptr) { if (b->idx.named.content_type != nullptr) {
if (!grpc_mdelem_eq_static( if (!grpc_mdelem_static_value_eq(
b->idx.named.content_type->md, b->idx.named.content_type->md,
GRPC_MDELEM_CONTENT_TYPE_APPLICATION_SLASH_GRPC)) { GRPC_MDELEM_CONTENT_TYPE_APPLICATION_SLASH_GRPC)) {
if (grpc_slice_buf_start_eq(GRPC_MDVALUE(b->idx.named.content_type->md), if (grpc_slice_buf_start_eq(GRPC_MDVALUE(b->idx.named.content_type->md),

@ -1281,8 +1281,8 @@ void grpc_chttp2_complete_closure_step(grpc_chttp2_transport* t,
static bool contains_non_ok_status(grpc_metadata_batch* batch) { static bool contains_non_ok_status(grpc_metadata_batch* batch) {
if (batch->idx.named.grpc_status != nullptr) { if (batch->idx.named.grpc_status != nullptr) {
return !grpc_mdelem_eq_static(batch->idx.named.grpc_status->md, return !grpc_mdelem_static_value_eq(batch->idx.named.grpc_status->md,
GRPC_MDELEM_GRPC_STATUS_0); GRPC_MDELEM_GRPC_STATUS_0);
} }
return false; return false;
} }

@ -124,7 +124,18 @@ grpc_mdelem grpc_mdelem_create(
const grpc_slice& key, const grpc_slice& value, const grpc_slice& key, const grpc_slice& value,
grpc_mdelem_data* compatible_external_backing_store); grpc_mdelem_data* compatible_external_backing_store);
#define GRPC_MDKEY(md) (GRPC_MDELEM_DATA(md)->key)
#define GRPC_MDVALUE(md) (GRPC_MDELEM_DATA(md)->value)
bool grpc_mdelem_eq(grpc_mdelem a, grpc_mdelem b); bool grpc_mdelem_eq(grpc_mdelem a, grpc_mdelem b);
/* Often we compare metadata where we know a-priori that the second parameter is
* static, and that the keys match. This most commonly happens when processing
* metadata batch callouts in initial/trailing filters. In this case, fastpath
* grpc_mdelem_eq and remove unnecessary checks. */
inline bool grpc_mdelem_static_value_eq(grpc_mdelem a, grpc_mdelem b_static) {
if (a.payload == b_static.payload) return true;
return grpc_slice_eq(GRPC_MDVALUE(a), GRPC_MDVALUE(b_static));
}
/* Mutator and accessor for grpc_mdelem user data. The destructor function /* Mutator and accessor for grpc_mdelem user data. The destructor function
is used as a type tag and is checked during user_data fetch. */ is used as a type tag and is checked during user_data fetch. */
@ -144,9 +155,6 @@ grpc_mdelem grpc_mdelem_ref(grpc_mdelem md);
void grpc_mdelem_unref(grpc_mdelem md); void grpc_mdelem_unref(grpc_mdelem md);
#endif #endif
#define GRPC_MDKEY(md) (GRPC_MDELEM_DATA(md)->key)
#define GRPC_MDVALUE(md) (GRPC_MDELEM_DATA(md)->value)
#define GRPC_MDNULL GRPC_MAKE_MDELEM(NULL, GRPC_MDELEM_STORAGE_EXTERNAL) #define GRPC_MDNULL GRPC_MAKE_MDELEM(NULL, GRPC_MDELEM_STORAGE_EXTERNAL)
#define GRPC_MDISNULL(md) (GRPC_MDELEM_DATA(md) == NULL) #define GRPC_MDISNULL(md) (GRPC_MDELEM_DATA(md) == NULL)
@ -160,9 +168,4 @@ void grpc_mdelem_unref(grpc_mdelem md);
void grpc_mdctx_global_init(void); void grpc_mdctx_global_init(void);
void grpc_mdctx_global_shutdown(); void grpc_mdctx_global_shutdown();
inline bool grpc_mdelem_eq_static(grpc_mdelem a_static, grpc_mdelem b_static) {
if (a_static.payload == b_static.payload) return true;
return grpc_slice_eq(GRPC_MDVALUE(a_static), GRPC_MDVALUE(b_static));
}
#endif /* GRPC_CORE_LIB_TRANSPORT_METADATA_H */ #endif /* GRPC_CORE_LIB_TRANSPORT_METADATA_H */

@ -31,13 +31,13 @@
static void destroy_status(void* ignored) {} static void destroy_status(void* ignored) {}
grpc_status_code grpc_get_status_code_from_metadata(grpc_mdelem md) { grpc_status_code grpc_get_status_code_from_metadata(grpc_mdelem md) {
if (grpc_mdelem_eq_static(md, GRPC_MDELEM_GRPC_STATUS_0)) { if (grpc_mdelem_static_value_eq(md, GRPC_MDELEM_GRPC_STATUS_0)) {
return GRPC_STATUS_OK; return GRPC_STATUS_OK;
} }
if (grpc_mdelem_eq_static(md, GRPC_MDELEM_GRPC_STATUS_1)) { if (grpc_mdelem_static_value_eq(md, GRPC_MDELEM_GRPC_STATUS_1)) {
return GRPC_STATUS_CANCELLED; return GRPC_STATUS_CANCELLED;
} }
if (grpc_mdelem_eq_static(md, GRPC_MDELEM_GRPC_STATUS_2)) { if (grpc_mdelem_static_value_eq(md, GRPC_MDELEM_GRPC_STATUS_2)) {
return GRPC_STATUS_UNKNOWN; return GRPC_STATUS_UNKNOWN;
} }
void* user_data = grpc_mdelem_get_user_data(md, destroy_status); void* user_data = grpc_mdelem_get_user_data(md, destroy_status);

Loading…
Cancel
Save