From f13abc071cb91c06e90f7121c66f39bae59ec691 Mon Sep 17 00:00:00 2001 From: Arjun Roy Date: Tue, 21 May 2019 19:02:27 -0700 Subject: [PATCH] Inlined and saved some ops for grpc_is_binary_header. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Various improvements for parsing/encoding: BM_HpackEncoderEncodeDeadline 171ns ± 0% 164ns ± 0% -3.74% (p=0.000 n=20+17) BM_HpackEncoderEncodeHeader/0/16384 102ns ± 1% 98ns ± 0% -3.28% (p=0.000 n=20+19) BM_HpackParserParseHeader 177ns ± 1% 165ns ± 1% -6.41% (p=0.000 n=19+18) BM_HpackParserParseHeader, UnrefHeader> 212ns ± 2% 199ns ± 1% -6.28% (p=0.000 n=20+18) --- .../chttp2/transport/hpack_encoder.cc | 18 +++-- .../chttp2/transport/hpack_parser.cc | 21 +++++- .../transport/chttp2/transport/hpack_table.cc | 7 +- .../cronet/transport/cronet_transport.cc | 5 +- src/core/lib/iomgr/error.cc | 6 +- src/core/lib/iomgr/error.h | 12 +++- .../credentials/plugin/plugin_credentials.cc | 2 +- src/core/lib/slice/slice_string_helpers.cc | 2 +- src/core/lib/slice/slice_string_helpers.h | 2 +- src/core/lib/surface/call.cc | 2 +- src/core/lib/surface/channel.cc | 57 +--------------- src/core/lib/surface/channel.h | 66 +++++++++++++++++-- src/core/lib/surface/validate_metadata.cc | 16 +++-- src/core/lib/surface/validate_metadata.h | 15 ++++- 14 files changed, 140 insertions(+), 91 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc index d2607e97707..b9871a3633e 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc @@ -37,6 +37,7 @@ #include "src/core/lib/debug/stats.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" +#include "src/core/lib/surface/validate_metadata.h" #include "src/core/lib/transport/metadata.h" #include "src/core/lib/transport/static_metadata.h" #include "src/core/lib/transport/timeout_encoding.h" @@ -323,9 +324,14 @@ typedef struct { bool insert_null_before_wire_value; } wire_value; +template static wire_value get_wire_value(grpc_mdelem elem, bool true_binary_enabled) { wire_value wire_val; - if (grpc_is_binary_header(GRPC_MDKEY(elem))) { + bool is_bin_hdr = + mdkey_definitely_interned + ? grpc_is_refcounted_slice_binary_header(GRPC_MDKEY(elem)) + : grpc_is_binary_header_internal(GRPC_MDKEY(elem)); + if (is_bin_hdr) { if (true_binary_enabled) { GRPC_STATS_INC_HPACK_SEND_BINARY(); wire_val.huffman_prefix = 0x00; @@ -363,7 +369,7 @@ static void emit_lithdr_incidx(grpc_chttp2_hpack_compressor* c, framer_state* st) { GRPC_STATS_INC_HPACK_SEND_LITHDR_INCIDX(); uint32_t len_pfx = GRPC_CHTTP2_VARINT_LENGTH(key_index, 2); - wire_value value = get_wire_value(elem, st->use_true_binary_metadata); + wire_value value = get_wire_value(elem, st->use_true_binary_metadata); size_t len_val = wire_value_length(value); uint32_t len_val_len; GPR_ASSERT(len_val <= UINT32_MAX); @@ -380,7 +386,7 @@ static void emit_lithdr_noidx(grpc_chttp2_hpack_compressor* c, framer_state* st) { GRPC_STATS_INC_HPACK_SEND_LITHDR_NOTIDX(); uint32_t len_pfx = GRPC_CHTTP2_VARINT_LENGTH(key_index, 4); - wire_value value = get_wire_value(elem, st->use_true_binary_metadata); + wire_value value = get_wire_value(elem, st->use_true_binary_metadata); size_t len_val = wire_value_length(value); uint32_t len_val_len; GPR_ASSERT(len_val <= UINT32_MAX); @@ -399,7 +405,7 @@ static void emit_lithdr_incidx_v(grpc_chttp2_hpack_compressor* c, GRPC_STATS_INC_HPACK_SEND_LITHDR_INCIDX_V(); GRPC_STATS_INC_HPACK_SEND_UNCOMPRESSED(); uint32_t len_key = static_cast GRPC_SLICE_LENGTH(GRPC_MDKEY(elem)); - wire_value value = get_wire_value(elem, st->use_true_binary_metadata); + wire_value value = get_wire_value(elem, st->use_true_binary_metadata); uint32_t len_val = static_cast(wire_value_length(value)); uint32_t len_key_len = GRPC_CHTTP2_VARINT_LENGTH(len_key, 1); uint32_t len_val_len = GRPC_CHTTP2_VARINT_LENGTH(len_val, 1); @@ -421,7 +427,7 @@ static void emit_lithdr_noidx_v(grpc_chttp2_hpack_compressor* c, GRPC_STATS_INC_HPACK_SEND_LITHDR_NOTIDX_V(); GRPC_STATS_INC_HPACK_SEND_UNCOMPRESSED(); uint32_t len_key = static_cast GRPC_SLICE_LENGTH(GRPC_MDKEY(elem)); - wire_value value = get_wire_value(elem, st->use_true_binary_metadata); + wire_value value = get_wire_value(elem, st->use_true_binary_metadata); uint32_t len_val = static_cast(wire_value_length(value)); uint32_t len_key_len = GRPC_CHTTP2_VARINT_LENGTH(len_key, 1); uint32_t len_val_len = GRPC_CHTTP2_VARINT_LENGTH(len_val, 1); @@ -464,7 +470,7 @@ static void hpack_enc(grpc_chttp2_hpack_compressor* c, grpc_mdelem elem, if (GRPC_TRACE_FLAG_ENABLED(grpc_http_trace)) { char* k = grpc_slice_to_c_string(GRPC_MDKEY(elem)); char* v = nullptr; - if (grpc_is_binary_header(GRPC_MDKEY(elem))) { + if (grpc_is_binary_header_internal(GRPC_MDKEY(elem))) { v = grpc_dump_slice(GRPC_MDVALUE(elem), GPR_DUMP_HEX); } else { v = grpc_slice_to_c_string(GRPC_MDVALUE(elem)); diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc index 6e422127511..7d5c39e5144 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -35,6 +35,7 @@ #include "src/core/lib/profiling/timers.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" +#include "src/core/lib/surface/validate_metadata.h" #include "src/core/lib/transport/http2_errors.h" typedef enum { @@ -627,7 +628,7 @@ static grpc_error* on_hdr(grpc_chttp2_hpack_parser* p, grpc_mdelem md, if (GRPC_TRACE_FLAG_ENABLED(grpc_http_trace)) { char* k = grpc_slice_to_c_string(GRPC_MDKEY(md)); char* v = nullptr; - if (grpc_is_binary_header(GRPC_MDKEY(md))) { + if (grpc_is_binary_header_internal(GRPC_MDKEY(md))) { v = grpc_dump_slice(GRPC_MDVALUE(md), GPR_DUMP_HEX); } else { v = grpc_slice_to_c_string(GRPC_MDVALUE(md)); @@ -1494,7 +1495,13 @@ static grpc_error* parse_key_string(grpc_chttp2_hpack_parser* p, /* check if a key represents a binary header or not */ static bool is_binary_literal_header(grpc_chttp2_hpack_parser* p) { - return grpc_is_binary_header( + /* We know that either argument here is a reference counter slice. + * 1. If a result of grpc_slice_from_static_buffer, the refcount is set to + * NoopRefcount. + * 2. If it's p->key.data.referenced, then p->key.copied was set to false, + * which occurs in begin_parse_string() - where the refcount is set to + * p->current_slice_refcount, which is not null. */ + return grpc_is_refcounted_slice_binary_header( p->key.copied ? grpc_slice_from_static_buffer(p->key.data.copied.str, p->key.data.copied.length) : p->key.data.referenced); @@ -1511,7 +1518,15 @@ static grpc_error* is_binary_indexed_header(grpc_chttp2_hpack_parser* p, static_cast(p->index)), GRPC_ERROR_INT_SIZE, static_cast(p->table.num_ents)); } - *is = grpc_is_binary_header(GRPC_MDKEY(elem)); + /* We know that GRPC_MDKEY(elem) points to a reference counted slice since: + * 1. elem was a result of grpc_chttp2_hptbl_lookup + * 2. An item in this table is either static (see entries with + * index < GRPC_CHTTP2_LAST_STATIC_ENTRY or added via + * grpc_chttp2_hptbl_add). + * 3. If added via grpc_chttp2_hptbl_add, the entry is either static or + * interned. + * 4. Both static and interned element slices have non-null refcounts. */ + *is = grpc_is_refcounted_slice_binary_header(GRPC_MDKEY(elem)); return GRPC_ERROR_NONE; } diff --git a/src/core/ext/transport/chttp2/transport/hpack_table.cc b/src/core/ext/transport/chttp2/transport/hpack_table.cc index 16aeb49df4d..f1cf4acf7dd 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_table.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_table.cc @@ -29,6 +29,7 @@ #include "src/core/lib/debug/trace.h" #include "src/core/lib/gpr/murmur_hash.h" +#include "src/core/lib/surface/validate_metadata.h" #include "src/core/lib/transport/static_metadata.h" extern grpc_core::TraceFlag grpc_http_trace; @@ -375,9 +376,11 @@ static size_t get_base64_encoded_size(size_t raw_length) { size_t grpc_chttp2_get_size_in_hpack_table(grpc_mdelem elem, bool use_true_binary_metadata) { - size_t overhead_and_key = 32 + GRPC_SLICE_LENGTH(GRPC_MDKEY(elem)); + const uint8_t* key_buf = GRPC_SLICE_START_PTR(GRPC_MDKEY(elem)); + size_t key_len = GRPC_SLICE_LENGTH(GRPC_MDKEY(elem)); + size_t overhead_and_key = 32 + key_len; size_t value_len = GRPC_SLICE_LENGTH(GRPC_MDVALUE(elem)); - if (grpc_is_binary_header(GRPC_MDKEY(elem))) { + if (grpc_key_is_binary_header(key_buf, key_len)) { return overhead_and_key + (use_true_binary_metadata ? value_len + 1 : get_base64_encoded_size(value_len)); diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index 413de807638..3ddda268cfb 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -38,6 +38,7 @@ #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" #include "src/core/lib/surface/channel.h" +#include "src/core/lib/surface/validate_metadata.h" #include "src/core/lib/transport/metadata_batch.h" #include "src/core/lib/transport/static_metadata.h" #include "src/core/lib/transport/transport_impl.h" @@ -419,7 +420,7 @@ static void convert_cronet_array_to_metadata( grpc_slice key = grpc_slice_intern( grpc_slice_from_static_string(header_array->headers[i].key)); grpc_slice value; - if (grpc_is_binary_header(key)) { + if (grpc_is_refcounted_slice_binary_header(key)) { value = grpc_slice_from_static_string(header_array->headers[i].value); value = grpc_slice_intern(grpc_chttp2_base64_decode_with_length( value, grpc_chttp2_base64_infer_length_after_decode(value))); @@ -746,7 +747,7 @@ static void convert_metadata_to_cronet_headers( curr = curr->next; char* key = grpc_slice_to_c_string(GRPC_MDKEY(mdelem)); char* value; - if (grpc_is_binary_header(GRPC_MDKEY(mdelem))) { + if (grpc_is_binary_header_internal(GRPC_MDKEY(mdelem))) { grpc_slice wire_value = grpc_chttp2_base64_encode(GRPC_MDVALUE(mdelem)); value = grpc_slice_to_c_string(wire_value); grpc_slice_unref_internal(wire_value); diff --git a/src/core/lib/iomgr/error.cc b/src/core/lib/iomgr/error.cc index f194eb62d48..ebec9dc704a 100644 --- a/src/core/lib/iomgr/error.cc +++ b/src/core/lib/iomgr/error.cc @@ -795,9 +795,9 @@ grpc_error* grpc_wsa_error(const char* file, int line, int err, } #endif -bool grpc_log_if_error(const char* what, grpc_error* error, const char* file, - int line) { - if (error == GRPC_ERROR_NONE) return true; +bool grpc_log_error(const char* what, grpc_error* error, const char* file, + int line) { + GPR_DEBUG_ASSERT(error != GRPC_ERROR_NONE); const char* msg = grpc_error_string(error); gpr_log(file, line, GPR_LOG_SEVERITY_ERROR, "%s: %s", what, msg); GRPC_ERROR_UNREF(error); diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index 7b04888e9ec..0a72e5ca101 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -261,9 +261,15 @@ grpc_error* grpc_wsa_error(const char* file, int line, int err, #define GRPC_WSA_ERROR(err, call_name) \ grpc_wsa_error(__FILE__, __LINE__, err, call_name) -bool grpc_log_if_error(const char* what, grpc_error* error, const char* file, - int line); +bool grpc_log_error(const char* what, grpc_error* error, const char* file, + int line); +inline bool grpc_log_if_error(const char* what, grpc_error* error, + const char* file, int line) { + return error == GRPC_ERROR_NONE ? true + : grpc_log_error(what, error, file, line); +} + #define GRPC_LOG_IF_ERROR(what, error) \ - grpc_log_if_error((what), (error), __FILE__, __LINE__) + (grpc_log_if_error((what), (error), __FILE__, __LINE__)) #endif /* GRPC_CORE_LIB_IOMGR_ERROR_H */ diff --git a/src/core/lib/security/credentials/plugin/plugin_credentials.cc b/src/core/lib/security/credentials/plugin/plugin_credentials.cc index 333366d0f0a..92af88aee0b 100644 --- a/src/core/lib/security/credentials/plugin/plugin_credentials.cc +++ b/src/core/lib/security/credentials/plugin/plugin_credentials.cc @@ -85,7 +85,7 @@ static grpc_error* process_plugin_result( grpc_validate_header_key_is_legal(md[i].key))) { seen_illegal_header = true; break; - } else if (!grpc_is_binary_header(md[i].key) && + } else if (!grpc_is_binary_header_internal(md[i].key) && !GRPC_LOG_IF_ERROR( "validate_metadata_from_plugin", grpc_validate_header_nonbin_value_is_legal(md[i].value))) { diff --git a/src/core/lib/slice/slice_string_helpers.cc b/src/core/lib/slice/slice_string_helpers.cc index 6af9c33eb56..c2392fd392f 100644 --- a/src/core/lib/slice/slice_string_helpers.cc +++ b/src/core/lib/slice/slice_string_helpers.cc @@ -27,7 +27,7 @@ #include "src/core/lib/gpr/string.h" #include "src/core/lib/slice/slice_internal.h" -char* grpc_dump_slice(grpc_slice s, uint32_t flags) { +char* grpc_dump_slice(const grpc_slice& s, uint32_t flags) { return gpr_dump(reinterpret_cast GRPC_SLICE_START_PTR(s), GRPC_SLICE_LENGTH(s), flags); } diff --git a/src/core/lib/slice/slice_string_helpers.h b/src/core/lib/slice/slice_string_helpers.h index 976f72411a8..cb1df658fa5 100644 --- a/src/core/lib/slice/slice_string_helpers.h +++ b/src/core/lib/slice/slice_string_helpers.h @@ -30,7 +30,7 @@ #include "src/core/lib/gpr/string.h" /* Calls gpr_dump on a slice. */ -char* grpc_dump_slice(grpc_slice slice, uint32_t flags); +char* grpc_dump_slice(const grpc_slice& slice, uint32_t flags); /** Split \a str by the separator \a sep. Results are stored in \a dst, which * should be a properly initialized instance. */ diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 254476f47e3..0ae7de29070 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -899,7 +899,7 @@ static int prepare_application_metadata(grpc_call* call, int count, if (!GRPC_LOG_IF_ERROR("validate_metadata", grpc_validate_header_key_is_legal(md->key))) { break; - } else if (!grpc_is_binary_header(md->key) && + } else if (!grpc_is_binary_header_internal(md->key) && !GRPC_LOG_IF_ERROR( "validate_metadata", grpc_validate_header_nonbin_value_is_legal(md->value))) { diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index e47cb4360ea..12869b7780d 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -59,23 +59,6 @@ typedef struct registered_call { struct registered_call* next; } registered_call; -struct grpc_channel { - int is_client; - grpc_compression_options compression_options; - - gpr_atm call_size_estimate; - grpc_resource_user* resource_user; - - gpr_mu registered_call_mu; - registered_call* registered_calls; - - grpc_core::RefCountedPtr channelz_channel; - - char* target; -}; - -#define CHANNEL_STACK_FROM_CHANNEL(c) ((grpc_channel_stack*)((c) + 1)) - static void destroy_channel(void* arg, grpc_error* error); grpc_channel* grpc_channel_create_with_builder( @@ -214,11 +197,6 @@ static grpc_channel_args* build_channel_args( return grpc_channel_args_copy_and_add(input_args, new_args, num_new_args); } -grpc_core::channelz::ChannelNode* grpc_channel_get_channelz_node( - grpc_channel* channel) { - return channel->channelz_channel.get(); -} - grpc_channel* grpc_channel_create(const char* target, const grpc_channel_args* input_args, grpc_channel_stack_type channel_stack_type, @@ -421,21 +399,6 @@ grpc_call* grpc_channel_create_registered_call( return call; } -#ifndef NDEBUG -#define REF_REASON reason -#define REF_ARG , const char* reason -#else -#define REF_REASON "" -#define REF_ARG -#endif -void grpc_channel_internal_ref(grpc_channel* c REF_ARG) { - GRPC_CHANNEL_STACK_REF(CHANNEL_STACK_FROM_CHANNEL(c), REF_REASON); -} - -void grpc_channel_internal_unref(grpc_channel* c REF_ARG) { - GRPC_CHANNEL_STACK_UNREF(CHANNEL_STACK_FROM_CHANNEL(c), REF_REASON); -} - static void destroy_channel(void* arg, grpc_error* error) { grpc_channel* channel = static_cast(arg); if (channel->channelz_channel != nullptr) { @@ -475,25 +438,9 @@ void grpc_channel_destroy(grpc_channel* channel) { GRPC_CHANNEL_INTERNAL_UNREF(channel, "channel"); } -grpc_channel_stack* grpc_channel_get_channel_stack(grpc_channel* channel) { - return CHANNEL_STACK_FROM_CHANNEL(channel); -} - -grpc_compression_options grpc_channel_compression_options( - const grpc_channel* channel) { - return channel->compression_options; -} - -grpc_mdelem grpc_channel_get_reffed_status_elem(grpc_channel* channel, int i) { +grpc_mdelem grpc_channel_get_reffed_status_elem_slowpath(grpc_channel* channel, + int i) { char tmp[GPR_LTOA_MIN_BUFSIZE]; - switch (i) { - case 0: - return GRPC_MDELEM_GRPC_STATUS_0; - case 1: - return GRPC_MDELEM_GRPC_STATUS_1; - case 2: - return GRPC_MDELEM_GRPC_STATUS_2; - } gpr_ltoa(i, tmp); return grpc_mdelem_from_slices(GRPC_MDSTR_GRPC_STATUS, grpc_slice_from_copied_string(tmp)); diff --git a/src/core/lib/surface/channel.h b/src/core/lib/surface/channel.h index ab00b8e94f7..5d666220745 100644 --- a/src/core/lib/surface/channel.h +++ b/src/core/lib/surface/channel.h @@ -59,22 +59,76 @@ grpc_core::channelz::ChannelNode* grpc_channel_get_channelz_node( status_code. The returned elem is owned by the caller. */ -grpc_mdelem grpc_channel_get_reffed_status_elem(grpc_channel* channel, - int status_code); +grpc_mdelem grpc_channel_get_reffed_status_elem_slowpath(grpc_channel* channel, + int status_code); +inline grpc_mdelem grpc_channel_get_reffed_status_elem(grpc_channel* channel, + int status_code) { + switch (status_code) { + case 0: + return GRPC_MDELEM_GRPC_STATUS_0; + case 1: + return GRPC_MDELEM_GRPC_STATUS_1; + case 2: + return GRPC_MDELEM_GRPC_STATUS_2; + } + return grpc_channel_get_reffed_status_elem_slowpath(channel, status_code); +} size_t grpc_channel_get_call_size_estimate(grpc_channel* channel); void grpc_channel_update_call_size_estimate(grpc_channel* channel, size_t size); +struct registered_call; +struct grpc_channel { + int is_client; + grpc_compression_options compression_options; + + gpr_atm call_size_estimate; + grpc_resource_user* resource_user; + + gpr_mu registered_call_mu; + registered_call* registered_calls; + + grpc_core::RefCountedPtr channelz_channel; + + char* target; +}; +#define CHANNEL_STACK_FROM_CHANNEL(c) ((grpc_channel_stack*)((c) + 1)) + +inline grpc_compression_options grpc_channel_compression_options( + const grpc_channel* channel) { + return channel->compression_options; +} + +inline grpc_channel_stack* grpc_channel_get_channel_stack( + grpc_channel* channel) { + return CHANNEL_STACK_FROM_CHANNEL(channel); +} + +inline grpc_core::channelz::ChannelNode* grpc_channel_get_channelz_node( + grpc_channel* channel) { + return channel->channelz_channel.get(); +} + #ifndef NDEBUG -void grpc_channel_internal_ref(grpc_channel* channel, const char* reason); -void grpc_channel_internal_unref(grpc_channel* channel, const char* reason); +inline void grpc_channel_internal_ref(grpc_channel* channel, + const char* reason) { + GRPC_CHANNEL_STACK_REF(CHANNEL_STACK_FROM_CHANNEL(channel), reason); +} +inline void grpc_channel_internal_unref(grpc_channel* channel, + const char* reason) { + GRPC_CHANNEL_STACK_UNREF(CHANNEL_STACK_FROM_CHANNEL(channel), reason); +} #define GRPC_CHANNEL_INTERNAL_REF(channel, reason) \ grpc_channel_internal_ref(channel, reason) #define GRPC_CHANNEL_INTERNAL_UNREF(channel, reason) \ grpc_channel_internal_unref(channel, reason) #else -void grpc_channel_internal_ref(grpc_channel* channel); -void grpc_channel_internal_unref(grpc_channel* channel); +inline void grpc_channel_internal_ref(grpc_channel* channel) { + GRPC_CHANNEL_STACK_REF(CHANNEL_STACK_FROM_CHANNEL(channel), "unused"); +} +inline void grpc_channel_internal_unref(grpc_channel* channel) { + GRPC_CHANNEL_STACK_UNREF(CHANNEL_STACK_FROM_CHANNEL(channel), "unused"); +} #define GRPC_CHANNEL_INTERNAL_REF(channel, reason) \ grpc_channel_internal_ref(channel) #define GRPC_CHANNEL_INTERNAL_UNREF(channel, reason) \ diff --git a/src/core/lib/surface/validate_metadata.cc b/src/core/lib/surface/validate_metadata.cc index 2dd18f3dd3f..a92ab823a38 100644 --- a/src/core/lib/surface/validate_metadata.cc +++ b/src/core/lib/surface/validate_metadata.cc @@ -29,7 +29,8 @@ #include "src/core/lib/slice/slice_string_helpers.h" #include "src/core/lib/surface/validate_metadata.h" -static grpc_error* conforms_to(grpc_slice slice, const uint8_t* legal_bits, +static grpc_error* conforms_to(const grpc_slice& slice, + const uint8_t* legal_bits, const char* err_desc) { const uint8_t* p = GRPC_SLICE_START_PTR(slice); const uint8_t* e = GRPC_SLICE_END_PTR(slice); @@ -57,7 +58,7 @@ static int error2int(grpc_error* error) { return r; } -grpc_error* grpc_validate_header_key_is_legal(grpc_slice slice) { +grpc_error* grpc_validate_header_key_is_legal(const grpc_slice& slice) { static const uint8_t legal_header_bits[256 / 8] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x60, 0xff, 0x03, 0x00, 0x00, 0x00, 0x80, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -77,7 +78,8 @@ int grpc_header_key_is_legal(grpc_slice slice) { return error2int(grpc_validate_header_key_is_legal(slice)); } -grpc_error* grpc_validate_header_nonbin_value_is_legal(grpc_slice slice) { +grpc_error* grpc_validate_header_nonbin_value_is_legal( + const grpc_slice& slice) { static const uint8_t legal_header_bits[256 / 8] = { 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -89,7 +91,11 @@ int grpc_header_nonbin_value_is_legal(grpc_slice slice) { return error2int(grpc_validate_header_nonbin_value_is_legal(slice)); } +int grpc_is_binary_header_internal(const grpc_slice& slice) { + return grpc_key_is_binary_header(GRPC_SLICE_START_PTR(slice), + GRPC_SLICE_LENGTH(slice)); +} + int grpc_is_binary_header(grpc_slice slice) { - if (GRPC_SLICE_LENGTH(slice) < 5) return 0; - return 0 == memcmp(GRPC_SLICE_END_PTR(slice) - 4, "-bin", 4); + return grpc_is_binary_header_internal(slice); } diff --git a/src/core/lib/surface/validate_metadata.h b/src/core/lib/surface/validate_metadata.h index e87fb7beedc..db3be684c11 100644 --- a/src/core/lib/surface/validate_metadata.h +++ b/src/core/lib/surface/validate_metadata.h @@ -24,7 +24,18 @@ #include #include "src/core/lib/iomgr/error.h" -grpc_error* grpc_validate_header_key_is_legal(grpc_slice slice); -grpc_error* grpc_validate_header_nonbin_value_is_legal(grpc_slice slice); +grpc_error* grpc_validate_header_key_is_legal(const grpc_slice& slice); +grpc_error* grpc_validate_header_nonbin_value_is_legal(const grpc_slice& slice); + +int grpc_is_binary_header_internal(const grpc_slice& slice); +inline int grpc_key_is_binary_header(const uint8_t* buf, size_t length) { + if (length < 5) return 0; + return 0 == memcmp(buf + length - 4, "-bin", 4); +} +inline int grpc_is_refcounted_slice_binary_header(const grpc_slice& slice) { + GPR_DEBUG_ASSERT(slice.refcount != nullptr); + return grpc_key_is_binary_header(slice.data.refcounted.bytes, + slice.data.refcounted.length); +} #endif /* GRPC_CORE_LIB_SURFACE_VALIDATE_METADATA_H */