From 2bb5bf208e422cf425ba12059698c6feacb449e6 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 8 May 2018 15:14:08 -0700 Subject: [PATCH] Remove GPR_UNLIKELY that are not in the call path --- .../filters/client_channel/backup_poller.cc | 2 +- .../client_channel/http_connect_handshaker.cc | 2 +- .../ext/filters/client_channel/http_proxy.cc | 6 ++-- .../filters/client_channel/parse_address.cc | 32 ++++++++----------- .../resolver/dns/c_ares/dns_resolver_ares.cc | 5 ++- .../resolver/sockaddr/sockaddr_resolver.cc | 2 +- .../client_channel/resolver_registry.cc | 2 +- .../ext/filters/client_channel/subchannel.cc | 2 +- .../filters/http/client/http_client_filter.cc | 6 ++-- .../chttp2/client/insecure/channel_create.cc | 2 +- .../client/secure/secure_channel_create.cc | 11 +++---- .../chttp2/server/insecure/server_chttp2.cc | 2 +- .../server/secure/server_secure_chttp2.cc | 2 +- .../chttp2/transport/chttp2_transport.cc | 4 +-- 14 files changed, 37 insertions(+), 43 deletions(-) diff --git a/src/core/ext/filters/client_channel/backup_poller.cc b/src/core/ext/filters/client_channel/backup_poller.cc index 895e255c067..3e2faa57bcf 100644 --- a/src/core/ext/filters/client_channel/backup_poller.cc +++ b/src/core/ext/filters/client_channel/backup_poller.cc @@ -61,7 +61,7 @@ static void init_globals() { char* env = gpr_getenv("GRPC_CLIENT_CHANNEL_BACKUP_POLL_INTERVAL_MS"); if (env != nullptr) { int poll_interval_ms = gpr_parse_nonnegative_int(env); - if (GPR_UNLIKELY(poll_interval_ms == -1)) { + if (poll_interval_ms == -1) { gpr_log(GPR_ERROR, "Invalid GRPC_CLIENT_CHANNEL_BACKUP_POLL_INTERVAL_MS: %s, " "default value %d will be used.", diff --git a/src/core/ext/filters/client_channel/http_connect_handshaker.cc b/src/core/ext/filters/client_channel/http_connect_handshaker.cc index 2106860d8e5..4e8b8b71dbd 100644 --- a/src/core/ext/filters/client_channel/http_connect_handshaker.cc +++ b/src/core/ext/filters/client_channel/http_connect_handshaker.cc @@ -280,7 +280,7 @@ static void http_connect_handshaker_do_handshake( gpr_malloc(sizeof(grpc_http_header) * num_header_strings)); for (size_t i = 0; i < num_header_strings; ++i) { char* sep = strchr(header_strings[i], ':'); - if (GPR_UNLIKELY(sep == nullptr)) { + if (sep == nullptr) { gpr_log(GPR_ERROR, "skipping unparseable HTTP CONNECT header: %s", header_strings[i]); continue; diff --git a/src/core/ext/filters/client_channel/http_proxy.cc b/src/core/ext/filters/client_channel/http_proxy.cc index 95c0c854988..29a6c0e3677 100644 --- a/src/core/ext/filters/client_channel/http_proxy.cc +++ b/src/core/ext/filters/client_channel/http_proxy.cc @@ -50,11 +50,11 @@ static char* get_http_proxy_server(char** user_cred) { size_t authority_nstrs; if (uri_str == nullptr) return nullptr; grpc_uri* uri = grpc_uri_parse(uri_str, false /* suppress_errors */); - if (GPR_UNLIKELY(uri == nullptr || uri->authority == nullptr)) { + if (uri == nullptr || uri->authority == nullptr) { gpr_log(GPR_ERROR, "cannot parse value of 'http_proxy' env var"); goto done; } - if (GPR_UNLIKELY(strcmp(uri->scheme, "http") != 0)) { + if (strcmp(uri->scheme, "http") != 0) { gpr_log(GPR_ERROR, "'%s' scheme not supported in proxy URI", uri->scheme); goto done; } @@ -93,7 +93,7 @@ static bool proxy_mapper_map_name(grpc_proxy_mapper* mapper, if (*name_to_resolve == nullptr) return false; char* no_proxy_str = nullptr; grpc_uri* uri = grpc_uri_parse(server_uri, false /* suppress_errors */); - if (GPR_UNLIKELY(uri == nullptr || uri->path[0] == '\0')) { + if (uri == nullptr || uri->path[0] == '\0') { gpr_log(GPR_ERROR, "'http_proxy' environment variable set, but cannot " "parse server URI '%s' -- not using proxy", diff --git a/src/core/ext/filters/client_channel/parse_address.cc b/src/core/ext/filters/client_channel/parse_address.cc index f3d6889f4da..b3900114ad9 100644 --- a/src/core/ext/filters/client_channel/parse_address.cc +++ b/src/core/ext/filters/client_channel/parse_address.cc @@ -39,7 +39,7 @@ bool grpc_parse_unix(const grpc_uri* uri, grpc_resolved_address* resolved_addr) { - if (GPR_UNLIKELY(strcmp("unix", uri->scheme) != 0)) { + if (strcmp("unix", uri->scheme) != 0) { gpr_log(GPR_ERROR, "Expected 'unix' scheme, got '%s'", uri->scheme); return false; } @@ -75,18 +75,17 @@ bool grpc_parse_ipv4_hostport(const char* hostport, grpc_resolved_address* addr, addr->len = static_cast(sizeof(grpc_sockaddr_in)); grpc_sockaddr_in* in = reinterpret_cast(addr->addr); in->sin_family = GRPC_AF_INET; - if (GPR_UNLIKELY(grpc_inet_pton(GRPC_AF_INET, host, &in->sin_addr) == 0)) { + if (grpc_inet_pton(GRPC_AF_INET, host, &in->sin_addr) == 0) { if (log_errors) gpr_log(GPR_ERROR, "invalid ipv4 address: '%s'", host); goto done; } // Parse port. - if (GPR_UNLIKELY(port == nullptr)) { + if (port == nullptr) { if (log_errors) gpr_log(GPR_ERROR, "no port given for ipv4 scheme"); goto done; } int port_num; - if (GPR_UNLIKELY(sscanf(port, "%d", &port_num) != 1 || port_num < 0 || - port_num > 65535)) { + if (sscanf(port, "%d", &port_num) != 1 || port_num < 0 || port_num > 65535) { if (log_errors) gpr_log(GPR_ERROR, "invalid ipv4 port: '%s'", port); goto done; } @@ -100,7 +99,7 @@ done: bool grpc_parse_ipv4(const grpc_uri* uri, grpc_resolved_address* resolved_addr) { - if (GPR_UNLIKELY(strcmp("ipv4", uri->scheme) != 0)) { + if (strcmp("ipv4", uri->scheme) != 0) { gpr_log(GPR_ERROR, "Expected 'ipv4' scheme, got '%s'", uri->scheme); return false; } @@ -131,35 +130,32 @@ bool grpc_parse_ipv6_hostport(const char* hostport, grpc_resolved_address* addr, uint32_t sin6_scope_id = 0; strncpy(host_without_scope, host, host_without_scope_len); host_without_scope[host_without_scope_len] = '\0'; - if (GPR_UNLIKELY(grpc_inet_pton(GRPC_AF_INET6, host_without_scope, - &in6->sin6_addr)) == 0) { + if (grpc_inet_pton(GRPC_AF_INET6, host_without_scope, &in6->sin6_addr) == + 0) { gpr_log(GPR_ERROR, "invalid ipv6 address: '%s'", host_without_scope); goto done; } - if (GPR_UNLIKELY( - gpr_parse_bytes_to_uint32(host_end + 1, - strlen(host) - host_without_scope_len - 1, - &sin6_scope_id) == 0)) { + if (gpr_parse_bytes_to_uint32(host_end + 1, + strlen(host) - host_without_scope_len - 1, + &sin6_scope_id) == 0) { gpr_log(GPR_ERROR, "invalid ipv6 scope id: '%s'", host_end + 1); goto done; } // Handle "sin6_scope_id" being type "u_long". See grpc issue #10027. in6->sin6_scope_id = sin6_scope_id; } else { - if (GPR_UNLIKELY(grpc_inet_pton(GRPC_AF_INET6, host, &in6->sin6_addr) == - 0)) { + if (grpc_inet_pton(GRPC_AF_INET6, host, &in6->sin6_addr) == 0) { gpr_log(GPR_ERROR, "invalid ipv6 address: '%s'", host); goto done; } } // Parse port. - if (GPR_UNLIKELY(port == nullptr)) { + if (port == nullptr) { if (log_errors) gpr_log(GPR_ERROR, "no port given for ipv6 scheme"); goto done; } int port_num; - if (GPR_UNLIKELY(sscanf(port, "%d", &port_num) != 1 || port_num < 0 || - port_num > 65535)) { + if (sscanf(port, "%d", &port_num) != 1 || port_num < 0 || port_num > 65535) { if (log_errors) gpr_log(GPR_ERROR, "invalid ipv6 port: '%s'", port); goto done; } @@ -173,7 +169,7 @@ done: bool grpc_parse_ipv6(const grpc_uri* uri, grpc_resolved_address* resolved_addr) { - if (GPR_UNLIKELY(strcmp("ipv6", uri->scheme) != 0)) { + if (strcmp("ipv6", uri->scheme) != 0) { gpr_log(GPR_ERROR, "Expected 'ipv6' scheme, got '%s'", uri->scheme); return false; } diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index 1b4767b47f2..c3c62b60bfe 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -225,15 +225,14 @@ bool ValueInJsonArray(grpc_json* array, const char* value) { char* ChooseServiceConfig(char* service_config_choice_json) { grpc_json* choices_json = grpc_json_parse_string(service_config_choice_json); - if (GPR_UNLIKELY(choices_json == nullptr || - choices_json->type != GRPC_JSON_ARRAY)) { + if (choices_json == nullptr || choices_json->type != GRPC_JSON_ARRAY) { gpr_log(GPR_ERROR, "cannot parse service config JSON string"); return nullptr; } char* service_config = nullptr; for (grpc_json* choice = choices_json->child; choice != nullptr; choice = choice->next) { - if (GPR_UNLIKELY(choice->type != GRPC_JSON_OBJECT)) { + if (choice->type != GRPC_JSON_OBJECT) { gpr_log(GPR_ERROR, "cannot parse service config JSON string"); break; } diff --git a/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc b/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc index 8f6132a7267..f74ac5aebe5 100644 --- a/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc @@ -123,7 +123,7 @@ void DoNothing(void* ignored) {} OrphanablePtr CreateSockaddrResolver( const ResolverArgs& args, bool parse(const grpc_uri* uri, grpc_resolved_address* dst)) { - if (GPR_UNLIKELY(0 != strcmp(args.uri->authority, ""))) { + if (0 != strcmp(args.uri->authority, "")) { gpr_log(GPR_ERROR, "authority-based URIs not supported by the %s scheme", args.uri->scheme); return OrphanablePtr(nullptr); diff --git a/src/core/ext/filters/client_channel/resolver_registry.cc b/src/core/ext/filters/client_channel/resolver_registry.cc index 7dfa57e11ae..91c0267f95e 100644 --- a/src/core/ext/filters/client_channel/resolver_registry.cc +++ b/src/core/ext/filters/client_channel/resolver_registry.cc @@ -74,7 +74,7 @@ class RegistryState { *uri = grpc_uri_parse(*canonical_target, 1); factory = *uri == nullptr ? nullptr : LookupResolverFactory((*uri)->scheme); - if (GPR_UNLIKELY(factory == nullptr)) { + if (factory == nullptr) { grpc_uri_destroy(grpc_uri_parse(target, 0)); grpc_uri_destroy(grpc_uri_parse(*canonical_target, 0)); gpr_log(GPR_ERROR, "don't know how to resolve '%s' or '%s'", target, diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 234ae498383..140441da108 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -577,7 +577,7 @@ static bool publish_transport_locked(grpc_subchannel* c) { grpc_error* error = grpc_channel_stack_builder_finish( builder, 0, 1, connection_destroy, nullptr, reinterpret_cast(&stk)); - if (GPR_UNLIKELY(error != GRPC_ERROR_NONE)) { + if (error != GRPC_ERROR_NONE) { grpc_transport_destroy(c->connecting_result.transport); gpr_log(GPR_ERROR, "error initializing subchannel stack: %s", grpc_error_string(error)); 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 8f27f2fea24..ae94ce47b9e 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -460,7 +460,7 @@ static size_t max_payload_size_from_args(const grpc_channel_args* args) { if (args != nullptr) { for (size_t i = 0; i < args->num_args; ++i) { if (0 == strcmp(args->args[i].key, GRPC_ARG_MAX_PAYLOAD_SIZE_FOR_GET)) { - if (GPR_UNLIKELY(args->args[i].type != GRPC_ARG_INTEGER)) { + if (args->args[i].type != GRPC_ARG_INTEGER) { gpr_log(GPR_ERROR, "%s: must be an integer", GRPC_ARG_MAX_PAYLOAD_SIZE_FOR_GET); } else { @@ -484,7 +484,7 @@ static grpc_slice user_agent_from_args(const grpc_channel_args* args, for (i = 0; args && i < args->num_args; i++) { if (0 == strcmp(args->args[i].key, GRPC_ARG_PRIMARY_USER_AGENT_STRING)) { - if (GPR_UNLIKELY(args->args[i].type != GRPC_ARG_STRING)) { + if (args->args[i].type != GRPC_ARG_STRING) { gpr_log(GPR_ERROR, "Channel argument '%s' should be a string", GRPC_ARG_PRIMARY_USER_AGENT_STRING); } else { @@ -503,7 +503,7 @@ static grpc_slice user_agent_from_args(const grpc_channel_args* args, for (i = 0; args && i < args->num_args; i++) { if (0 == strcmp(args->args[i].key, GRPC_ARG_SECONDARY_USER_AGENT_STRING)) { - if (GPR_UNLIKELY(args->args[i].type != GRPC_ARG_STRING)) { + if (args->args[i].type != GRPC_ARG_STRING) { gpr_log(GPR_ERROR, "Channel argument '%s' should be a string", GRPC_ARG_SECONDARY_USER_AGENT_STRING); } else { diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc index 62ec6353781..e6c8c382607 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc @@ -54,7 +54,7 @@ static grpc_subchannel* client_channel_factory_create_subchannel( static grpc_channel* client_channel_factory_create_channel( grpc_client_channel_factory* cc_factory, const char* target, grpc_client_channel_type type, const grpc_channel_args* args) { - if (GPR_UNLIKELY(target == nullptr)) { + if (target == nullptr) { gpr_log(GPR_ERROR, "cannot create channel with NULL target name"); return nullptr; } diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc index 5928ed876c8..5ce73a95d76 100644 --- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc +++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc @@ -50,15 +50,14 @@ static grpc_subchannel_args* get_secure_naming_subchannel_args( const grpc_subchannel_args* args) { grpc_channel_credentials* channel_credentials = grpc_channel_credentials_find_in_args(args->args); - if (GPR_UNLIKELY(channel_credentials == nullptr)) { + if (channel_credentials == nullptr) { gpr_log(GPR_ERROR, "Can't create subchannel: channel credentials missing for secure " "channel."); return nullptr; } // Make sure security connector does not already exist in args. - if (GPR_UNLIKELY(grpc_security_connector_find_in_args(args->args) != - nullptr)) { + if (grpc_security_connector_find_in_args(args->args) != nullptr) { gpr_log(GPR_ERROR, "Can't create subchannel: security connector already present in " "channel args."); @@ -118,7 +117,7 @@ static grpc_subchannel_args* get_secure_naming_subchannel_args( grpc_channel_credentials_create_security_connector( channel_credentials, authority.get(), args_with_authority, &subchannel_security_connector, &new_args_from_connector); - if (GPR_UNLIKELY(security_status != GRPC_SECURITY_OK)) { + if (security_status != GRPC_SECURITY_OK) { gpr_log(GPR_ERROR, "Failed to create secure subchannel for secure name '%s'", authority.get()); @@ -150,7 +149,7 @@ static grpc_subchannel* client_channel_factory_create_subchannel( grpc_client_channel_factory* cc_factory, const grpc_subchannel_args* args) { grpc_subchannel_args* subchannel_args = get_secure_naming_subchannel_args(args); - if (GPR_UNLIKELY(subchannel_args == nullptr)) { + if (subchannel_args == nullptr) { gpr_log( GPR_ERROR, "Failed to create subchannel arguments during subchannel creation."); @@ -168,7 +167,7 @@ static grpc_subchannel* client_channel_factory_create_subchannel( static grpc_channel* client_channel_factory_create_channel( grpc_client_channel_factory* cc_factory, const char* target, grpc_client_channel_type type, const grpc_channel_args* args) { - if (GPR_UNLIKELY(target == nullptr)) { + if (target == nullptr) { gpr_log(GPR_ERROR, "cannot create channel with NULL target name"); return nullptr; } diff --git a/src/core/ext/transport/chttp2/server/insecure/server_chttp2.cc b/src/core/ext/transport/chttp2/server/insecure/server_chttp2.cc index b4ec88a8349..99f18cdf39f 100644 --- a/src/core/ext/transport/chttp2/server/insecure/server_chttp2.cc +++ b/src/core/ext/transport/chttp2/server/insecure/server_chttp2.cc @@ -35,7 +35,7 @@ int grpc_server_add_insecure_http2_port(grpc_server* server, const char* addr) { grpc_error* err = grpc_chttp2_server_add_port( server, addr, grpc_channel_args_copy(grpc_server_get_channel_args(server)), &port_num); - if (GPR_UNLIKELY(err != GRPC_ERROR_NONE)) { + if (err != GRPC_ERROR_NONE) { const char* msg = grpc_error_string(err); gpr_log(GPR_ERROR, "%s", msg); diff --git a/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.cc b/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.cc index bc6b7a46f97..6689a17da63 100644 --- a/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.cc +++ b/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.cc @@ -79,7 +79,7 @@ done: GRPC_SECURITY_CONNECTOR_UNREF(&sc->base, "server"); } - if (GPR_UNLIKELY(err != GRPC_ERROR_NONE)) { + if (err != GRPC_ERROR_NONE) { const char* msg = grpc_error_string(err); gpr_log(GPR_ERROR, "%s", msg); diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index ebd66d5256c..7aa7be4b4b5 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -368,7 +368,7 @@ static void init_transport(grpc_chttp2_transport* t, const int value = grpc_channel_arg_get_integer(&channel_args->args[i], options); if (value >= 0) { - if (GPR_UNLIKELY((t->next_stream_id & 1) != (value & 1))) { + if ((t->next_stream_id & 1) != (value & 1)) { gpr_log(GPR_ERROR, "%s: low bit must be %d on %s", GRPC_ARG_HTTP2_INITIAL_SEQUENCE_NUMBER, t->next_stream_id & 1, is_client ? "client" : "server"); @@ -447,7 +447,7 @@ static void init_transport(grpc_chttp2_transport* t, grpc_channel_arg_get_integer(&channel_args->args[i], {0, 0, 1})); } else if (0 == strcmp(channel_args->args[i].key, GRPC_ARG_OPTIMIZATION_TARGET)) { - if (GPR_UNLIKELY(channel_args->args[i].type != GRPC_ARG_STRING)) { + if (channel_args->args[i].type != GRPC_ARG_STRING) { gpr_log(GPR_ERROR, "%s should be a string", GRPC_ARG_OPTIMIZATION_TARGET); } else if (0 == strcmp(channel_args->args[i].value.string, "blend")) {