From 7ea8a60ed0ba4faeeb912e9b76ae1d0a222b3ddf Mon Sep 17 00:00:00 2001 From: Noah Eisen Date: Thu, 14 Jun 2018 11:43:18 -0400 Subject: [PATCH] Revert "Add Type Checking On Channel Args" --- .../filters/client_channel/client_channel.cc | 71 ++++++++++-------- .../client_channel/http_connect_handshaker.cc | 9 ++- .../ext/filters/client_channel/http_proxy.cc | 4 +- .../client_channel/lb_policy/grpclb/grpclb.cc | 38 +++++----- .../lb_policy/grpclb/grpclb_channel_secure.cc | 7 +- .../lb_policy/pick_first/pick_first.cc | 8 +- .../lb_policy/round_robin/round_robin.cc | 8 +- .../client_channel/lb_policy_factory.cc | 7 +- .../resolver/dns/c_ares/dns_resolver_ares.cc | 13 ++-- .../resolver/dns/native/dns_resolver.cc | 7 +- .../resolver/fake/fake_resolver.cc | 15 ++-- .../ext/filters/client_channel/subchannel.cc | 5 +- .../ext/filters/deadline/deadline_filter.cc | 4 +- .../filters/http/client/http_client_filter.cc | 74 +++++++++++++------ .../ext/filters/http/http_filters_plugin.cc | 4 +- .../server_load_reporting_plugin.cc | 3 +- .../ext/filters/max_age/max_age_filter.cc | 13 ++-- .../message_size/message_size_filter.cc | 5 +- .../ext/transport/chttp2/client/authority.cc | 5 +- .../client/secure/secure_channel_create.cc | 5 +- .../chttp2/transport/chttp2_transport.cc | 17 +++-- .../cronet/transport/cronet_transport.cc | 16 +++- .../ext/transport/inproc/inproc_transport.cc | 7 +- src/core/lib/channel/channel_args.h | 37 ---------- src/core/lib/iomgr/resource_quota.cc | 16 +++- src/core/lib/iomgr/tcp_client_posix.cc | 16 ++-- src/core/lib/iomgr/tcp_server_custom.cc | 19 ++++- src/core/lib/iomgr/tcp_server_posix.cc | 19 ++++- .../iomgr/tcp_server_utils_posix_common.cc | 18 +++-- src/core/lib/iomgr/udp_server.cc | 10 ++- .../lib/security/context/security_context.cc | 21 +++++- .../lib/security/context/security_context.h | 1 + .../lib/security/credentials/credentials.cc | 45 +++++++++-- .../lib/security/credentials/credentials.h | 5 ++ .../credentials/fake/fake_credentials.cc | 5 +- .../google_default_credentials.cc | 11 ++- .../credentials/ssl/ssl_credentials.cc | 10 ++- .../security_connector/security_connector.cc | 22 +++++- .../security_connector/security_connector.h | 3 + .../transport/target_authority_table.cc | 13 +++- test/core/end2end/fixtures/h2_http_proxy.cc | 5 +- .../end2end/fixtures/http_proxy_fixture.cc | 3 +- .../sanity/check_channel_arg_usage.py | 55 -------------- 43 files changed, 399 insertions(+), 280 deletions(-) delete mode 100755 tools/run_tests/sanity/check_channel_arg_usage.py diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index f141aabe70b..ea6775a8d85 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -327,14 +327,16 @@ static void on_resolver_result_changed_locked(void* arg, grpc_error* error) { if (chand->resolver_result != nullptr) { if (chand->resolver != nullptr) { // Find LB policy name. - const char* lb_policy_name = grpc_channel_args_get_string( + const grpc_arg* channel_arg = grpc_channel_args_find( chand->resolver_result, GRPC_ARG_LB_POLICY_NAME); + const char* lb_policy_name = grpc_channel_arg_get_string(channel_arg); // Special case: If at least one balancer address is present, we use // the grpclb policy, regardless of what the resolver actually specified. - grpc_lb_addresses* addresses = - grpc_channel_args_get_pointer( - chand->resolver_result, GRPC_ARG_LB_ADDRESSES); - if (addresses != nullptr) { + channel_arg = + grpc_channel_args_find(chand->resolver_result, GRPC_ARG_LB_ADDRESSES); + if (channel_arg != nullptr && channel_arg->type == GRPC_ARG_POINTER) { + grpc_lb_addresses* addresses = + static_cast(channel_arg->value.pointer.p); bool found_balancer_address = false; for (size_t i = 0; i < addresses->num_addresses; ++i) { if (addresses->addresses[i].is_balancer) { @@ -398,15 +400,18 @@ static void on_resolver_result_changed_locked(void* arg, grpc_error* error) { // The copy will be saved in chand->lb_policy_name below. lb_policy_name_dup = gpr_strdup(lb_policy_name); // Find service config. - service_config_json = gpr_strdup(grpc_channel_args_get_string( - chand->resolver_result, GRPC_ARG_SERVICE_CONFIG)); + channel_arg = grpc_channel_args_find(chand->resolver_result, + GRPC_ARG_SERVICE_CONFIG); + service_config_json = + gpr_strdup(grpc_channel_arg_get_string(channel_arg)); if (service_config_json != nullptr) { grpc_core::UniquePtr service_config = grpc_core::ServiceConfig::Create(service_config_json); if (service_config != nullptr) { if (chand->enable_retries) { - const char* server_uri = grpc_channel_args_get_string( - chand->resolver_result, GRPC_ARG_SERVER_URI); + channel_arg = grpc_channel_args_find(chand->resolver_result, + GRPC_ARG_SERVER_URI); + const char* server_uri = grpc_channel_arg_get_string(channel_arg); GPR_ASSERT(server_uri != nullptr); grpc_uri* uri = grpc_uri_parse(server_uri, true); GPR_ASSERT(uri->path[0] != '\0'); @@ -643,37 +648,45 @@ static grpc_error* cc_init_channel_elem(grpc_channel_element* elem, "client_channel"); grpc_client_channel_start_backup_polling(chand->interested_parties); // Record max per-RPC retry buffer size. - chand->per_rpc_retry_buffer_size = (size_t)grpc_channel_args_get_integer( - args->channel_args, GRPC_ARG_PER_RPC_RETRY_BUFFER_SIZE, - {DEFAULT_PER_RPC_RETRY_BUFFER_SIZE, 0, INT_MAX}); + const grpc_arg* arg = grpc_channel_args_find( + args->channel_args, GRPC_ARG_PER_RPC_RETRY_BUFFER_SIZE); + chand->per_rpc_retry_buffer_size = (size_t)grpc_channel_arg_get_integer( + arg, {DEFAULT_PER_RPC_RETRY_BUFFER_SIZE, 0, INT_MAX}); // Record enable_retries. - chand->enable_retries = grpc_channel_args_get_bool( - args->channel_args, GRPC_ARG_ENABLE_RETRIES, true); + arg = grpc_channel_args_find(args->channel_args, GRPC_ARG_ENABLE_RETRIES); + chand->enable_retries = grpc_channel_arg_get_bool(arg, true); // Record client channel factory. - grpc_client_channel_factory* client_channel_factory = - grpc_channel_args_get_pointer( - args->channel_args, GRPC_ARG_CLIENT_CHANNEL_FACTORY); - if (client_channel_factory == nullptr) { + arg = grpc_channel_args_find(args->channel_args, + GRPC_ARG_CLIENT_CHANNEL_FACTORY); + if (arg == nullptr) { return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Missing or malformed client channel factory in args for client " - "channel filter"); + "Missing client channel factory in args for client channel filter"); } - grpc_client_channel_factory_ref(client_channel_factory); - chand->client_channel_factory = client_channel_factory; + if (arg->type != GRPC_ARG_POINTER) { + return GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "client channel factory arg must be a pointer"); + } + grpc_client_channel_factory_ref( + static_cast(arg->value.pointer.p)); + chand->client_channel_factory = + static_cast(arg->value.pointer.p); // Get server name to resolve, using proxy mapper if needed. - char* server_uri = - grpc_channel_args_get_string(args->channel_args, GRPC_ARG_SERVER_URI); - if (server_uri == nullptr) { + arg = grpc_channel_args_find(args->channel_args, GRPC_ARG_SERVER_URI); + if (arg == nullptr) { + return GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "Missing server uri in args for client channel filter"); + } + if (arg->type != GRPC_ARG_STRING) { return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Missing or malformed server uri in args for client channel filter"); + "server uri arg must be a string"); } char* proxy_name = nullptr; grpc_channel_args* new_args = nullptr; - grpc_proxy_mappers_map_name(server_uri, args->channel_args, &proxy_name, - &new_args); + grpc_proxy_mappers_map_name(arg->value.string, args->channel_args, + &proxy_name, &new_args); // Instantiate resolver. chand->resolver = grpc_core::ResolverRegistry::CreateResolver( - proxy_name != nullptr ? proxy_name : server_uri, + proxy_name != nullptr ? proxy_name : arg->value.string, new_args != nullptr ? new_args : args->channel_args, chand->interested_parties, chand->combiner); if (proxy_name != nullptr) gpr_free(proxy_name); 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 58f059829ca..4e8b8b71dbd 100644 --- a/src/core/ext/filters/client_channel/http_connect_handshaker.cc +++ b/src/core/ext/filters/client_channel/http_connect_handshaker.cc @@ -254,8 +254,9 @@ static void http_connect_handshaker_do_handshake( reinterpret_cast(handshaker_in); // Check for HTTP CONNECT channel arg. // If not found, invoke on_handshake_done without doing anything. - char* server_name = - grpc_channel_args_get_string(args->args, GRPC_ARG_HTTP_CONNECT_SERVER); + const grpc_arg* arg = + grpc_channel_args_find(args->args, GRPC_ARG_HTTP_CONNECT_SERVER); + char* server_name = grpc_channel_arg_get_string(arg); if (server_name == nullptr) { // Set shutdown to true so that subsequent calls to // http_connect_handshaker_shutdown() do nothing. @@ -266,8 +267,8 @@ static void http_connect_handshaker_do_handshake( return; } // Get headers from channel args. - char* arg_header_string = - grpc_channel_args_get_string(args->args, GRPC_ARG_HTTP_CONNECT_HEADERS); + arg = grpc_channel_args_find(args->args, GRPC_ARG_HTTP_CONNECT_HEADERS); + char* arg_header_string = grpc_channel_arg_get_string(arg); grpc_http_header* headers = nullptr; size_t num_headers = 0; char** header_strings = nullptr; diff --git a/src/core/ext/filters/client_channel/http_proxy.cc b/src/core/ext/filters/client_channel/http_proxy.cc index e21de35a7d2..9baccd8628b 100644 --- a/src/core/ext/filters/client_channel/http_proxy.cc +++ b/src/core/ext/filters/client_channel/http_proxy.cc @@ -88,7 +88,9 @@ done: * should be used. */ bool http_proxy_enabled(const grpc_channel_args* args) { - return grpc_channel_args_get_bool(args, GRPC_ARG_ENABLE_HTTP_PROXY, true); + const grpc_arg* arg = + grpc_channel_args_find(args, GRPC_ARG_ENABLE_HTTP_PROXY); + return grpc_channel_arg_get_bool(arg, true); } static bool proxy_mapper_map_name(grpc_proxy_mapper* mapper, diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 0ee4958f3d9..263b51ae895 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -1045,8 +1045,8 @@ GrpcLb::GrpcLb(const grpc_lb_addresses* addresses, grpc_combiner_scheduler(args.combiner)); grpc_connectivity_state_init(&state_tracker_, GRPC_CHANNEL_IDLE, "grpclb"); // Record server name. - const char* server_uri = - grpc_channel_args_get_string(args.args, GRPC_ARG_SERVER_URI); + const grpc_arg* arg = grpc_channel_args_find(args.args, GRPC_ARG_SERVER_URI); + const char* server_uri = grpc_channel_arg_get_string(arg); GPR_ASSERT(server_uri != nullptr); grpc_uri* uri = grpc_uri_parse(server_uri, true); GPR_ASSERT(uri->path[0] != '\0'); @@ -1058,12 +1058,12 @@ GrpcLb::GrpcLb(const grpc_lb_addresses* addresses, } grpc_uri_destroy(uri); // Record LB call timeout. - lb_call_timeout_ms_ = grpc_channel_args_get_integer( - args.args, GRPC_ARG_GRPCLB_CALL_TIMEOUT_MS, {0, 0, INT_MAX}); + arg = grpc_channel_args_find(args.args, GRPC_ARG_GRPCLB_CALL_TIMEOUT_MS); + lb_call_timeout_ms_ = grpc_channel_arg_get_integer(arg, {0, 0, INT_MAX}); // Record fallback timeout. - lb_fallback_timeout_ms_ = grpc_channel_args_get_integer( - args.args, GRPC_ARG_GRPCLB_FALLBACK_TIMEOUT_MS, - {GRPC_GRPCLB_DEFAULT_FALLBACK_TIMEOUT_MS, 0, INT_MAX}); + arg = grpc_channel_args_find(args.args, GRPC_ARG_GRPCLB_FALLBACK_TIMEOUT_MS); + lb_fallback_timeout_ms_ = grpc_channel_arg_get_integer( + arg, {GRPC_GRPCLB_DEFAULT_FALLBACK_TIMEOUT_MS, 0, INT_MAX}); // Process channel args. ProcessChannelArgsLocked(*args.args); } @@ -1284,10 +1284,8 @@ void GrpcLb::NotifyOnStateChangeLocked(grpc_connectivity_state* current, } void GrpcLb::ProcessChannelArgsLocked(const grpc_channel_args& args) { - const grpc_lb_addresses* addresses = - grpc_channel_args_get_pointer(&args, - GRPC_ARG_LB_ADDRESSES); - if (GPR_UNLIKELY(addresses == nullptr)) { + const grpc_arg* arg = grpc_channel_args_find(&args, GRPC_ARG_LB_ADDRESSES); + if (GPR_UNLIKELY(arg == nullptr || arg->type != GRPC_ARG_POINTER)) { // Ignore this update. gpr_log( GPR_ERROR, @@ -1295,6 +1293,8 @@ void GrpcLb::ProcessChannelArgsLocked(const grpc_channel_args& args) { this); return; } + const grpc_lb_addresses* addresses = + static_cast(arg->value.pointer.p); // Update fallback address list. if (fallback_backend_addresses_ != nullptr) { grpc_lb_addresses_destroy(fallback_backend_addresses_); @@ -1860,12 +1860,13 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { OrphanablePtr CreateLoadBalancingPolicy( const LoadBalancingPolicy::Args& args) const override { /* Count the number of gRPC-LB addresses. There must be at least one. */ - grpc_lb_addresses* addresses = - grpc_channel_args_get_pointer(args.args, - GRPC_ARG_LB_ADDRESSES); - if (addresses == nullptr) { + const grpc_arg* arg = + grpc_channel_args_find(args.args, GRPC_ARG_LB_ADDRESSES); + if (arg == nullptr || arg->type != GRPC_ARG_POINTER) { return nullptr; } + grpc_lb_addresses* addresses = + static_cast(arg->value.pointer.p); size_t num_grpclb_addrs = 0; for (size_t i = 0; i < addresses->num_addresses; ++i) { if (addresses->addresses[i].is_balancer) ++num_grpclb_addrs; @@ -1892,9 +1893,10 @@ bool maybe_add_client_load_reporting_filter(grpc_channel_stack_builder* builder, void* arg) { const grpc_channel_args* args = grpc_channel_stack_builder_get_channel_arguments(builder); - const char* lb_policy = - grpc_channel_args_get_string(args, GRPC_ARG_LB_POLICY_NAME); - if (lb_policy != nullptr && strcmp(lb_policy, "grpclb") == 0) { + const grpc_arg* channel_arg = + grpc_channel_args_find(args, GRPC_ARG_LB_POLICY_NAME); + if (channel_arg != nullptr && channel_arg->type == GRPC_ARG_STRING && + strcmp(channel_arg->value.string, "grpclb") == 0) { return grpc_channel_stack_builder_append_filter( builder, (const grpc_channel_filter*)arg, nullptr, nullptr); } diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc index 972bdd40d5f..441efd5e23b 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc @@ -72,10 +72,11 @@ grpc_channel_args* grpc_lb_policy_grpclb_modify_lb_channel_args( grpc_arg args_to_add[2]; size_t num_args_to_add = 0; // Add arg for targets info table. + const grpc_arg* arg = grpc_channel_args_find(args, GRPC_ARG_LB_ADDRESSES); + GPR_ASSERT(arg != nullptr); + GPR_ASSERT(arg->type == GRPC_ARG_POINTER); grpc_lb_addresses* addresses = - grpc_channel_args_get_pointer(args, - GRPC_ARG_LB_ADDRESSES); - GPR_ASSERT(addresses != nullptr); + static_cast(arg->value.pointer.p); grpc_core::RefCountedPtr target_authority_table = grpc_core::CreateTargetAuthorityTable(addresses); args_to_add[num_args_to_add++] = diff --git a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc index 70220e28d32..ff2140e628a 100644 --- a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc +++ b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc @@ -281,10 +281,8 @@ void PickFirst::PingOneLocked(grpc_closure* on_initiate, grpc_closure* on_ack) { } void PickFirst::UpdateLocked(const grpc_channel_args& args) { - const grpc_lb_addresses* addresses = - grpc_channel_args_get_pointer( - &args, GRPC_ARG_LB_ADDRESSES); - if (addresses == nullptr) { + const grpc_arg* arg = grpc_channel_args_find(&args, GRPC_ARG_LB_ADDRESSES); + if (arg == nullptr || arg->type != GRPC_ARG_POINTER) { if (subchannel_list_ == nullptr) { // If we don't have a current subchannel list, go into TRANSIENT FAILURE. grpc_connectivity_state_set( @@ -300,6 +298,8 @@ void PickFirst::UpdateLocked(const grpc_channel_args& args) { } return; } + const grpc_lb_addresses* addresses = + static_cast(arg->value.pointer.p); if (grpc_lb_pick_first_trace.enabled()) { gpr_log(GPR_INFO, "Pick First %p received update with %" PRIuPTR " addresses", this, diff --git a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc index 066f6ef7a68..b1773850653 100644 --- a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc +++ b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc @@ -607,10 +607,8 @@ void RoundRobin::PingOneLocked(grpc_closure* on_initiate, } void RoundRobin::UpdateLocked(const grpc_channel_args& args) { - grpc_lb_addresses* addresses = - grpc_channel_args_get_pointer(&args, - GRPC_ARG_LB_ADDRESSES); - if (GPR_UNLIKELY(addresses == nullptr)) { + const grpc_arg* arg = grpc_channel_args_find(&args, GRPC_ARG_LB_ADDRESSES); + if (GPR_UNLIKELY(arg == nullptr || arg->type != GRPC_ARG_POINTER)) { gpr_log(GPR_ERROR, "[RR %p] update provided no addresses; ignoring", this); // If we don't have a current subchannel list, go into TRANSIENT_FAILURE. // Otherwise, keep using the current subchannel list (ignore this update). @@ -622,6 +620,8 @@ void RoundRobin::UpdateLocked(const grpc_channel_args& args) { } return; } + grpc_lb_addresses* addresses = + static_cast(arg->value.pointer.p); if (grpc_lb_round_robin_trace.enabled()) { gpr_log(GPR_INFO, "[RR %p] received update with %" PRIuPTR " addresses", this, addresses->num_addresses); diff --git a/src/core/ext/filters/client_channel/lb_policy_factory.cc b/src/core/ext/filters/client_channel/lb_policy_factory.cc index 93dae753bff..7c8cba55b74 100644 --- a/src/core/ext/filters/client_channel/lb_policy_factory.cc +++ b/src/core/ext/filters/client_channel/lb_policy_factory.cc @@ -147,6 +147,9 @@ grpc_arg grpc_lb_addresses_create_channel_arg( grpc_lb_addresses* grpc_lb_addresses_find_channel_arg( const grpc_channel_args* channel_args) { - return grpc_channel_args_get_pointer( - channel_args, GRPC_ARG_LB_ADDRESSES); + const grpc_arg* lb_addresses_arg = + grpc_channel_args_find(channel_args, GRPC_ARG_LB_ADDRESSES); + if (lb_addresses_arg == nullptr || lb_addresses_arg->type != GRPC_ARG_POINTER) + return nullptr; + return static_cast(lb_addresses_arg->value.pointer.p); } 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 01156b1e406..f4f6444c5fa 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 @@ -140,11 +140,14 @@ AresDnsResolver::AresDnsResolver(const ResolverArgs& args) dns_server_ = gpr_strdup(args.uri->authority); } channel_args_ = grpc_channel_args_copy(args.args); - request_service_config_ = !grpc_channel_args_get_bool( - channel_args_, GRPC_ARG_SERVICE_CONFIG_DISABLE_RESOLUTION, false); - min_time_between_resolutions_ = grpc_channel_args_get_integer( - channel_args_, GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS, - {1000, 0, INT_MAX}); + const grpc_arg* arg = grpc_channel_args_find( + channel_args_, GRPC_ARG_SERVICE_CONFIG_DISABLE_RESOLUTION); + request_service_config_ = !grpc_channel_arg_get_integer( + arg, (grpc_integer_options){false, false, true}); + arg = grpc_channel_args_find(channel_args_, + GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS); + min_time_between_resolutions_ = + grpc_channel_arg_get_integer(arg, {1000, 0, INT_MAX}); interested_parties_ = grpc_pollset_set_create(); if (args.pollset_set != nullptr) { grpc_pollset_set_add_pollset_set(interested_parties_, args.pollset_set); diff --git a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc index 25e92a078d2..fae4c33a178 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc @@ -116,9 +116,10 @@ NativeDnsResolver::NativeDnsResolver(const ResolverArgs& args) if (path[0] == '/') ++path; name_to_resolve_ = gpr_strdup(path); channel_args_ = grpc_channel_args_copy(args.args); - min_time_between_resolutions_ = grpc_channel_args_get_integer( - args.args, GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS, - {1000, 0, INT_MAX}); + const grpc_arg* arg = grpc_channel_args_find( + args.args, GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS); + min_time_between_resolutions_ = + grpc_channel_arg_get_integer(arg, {1000, 0, INT_MAX}); interested_parties_ = grpc_pollset_set_create(); if (args.pollset_set != nullptr) { grpc_pollset_set_add_pollset_set(interested_parties_, args.pollset_set); diff --git a/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc b/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc index 9250dbd6b33..99a33f2277e 100644 --- a/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc @@ -252,15 +252,20 @@ static const grpc_arg_pointer_vtable response_generator_arg_vtable = { grpc_arg FakeResolverResponseGenerator::MakeChannelArg( FakeResolverResponseGenerator* generator) { - return grpc_channel_arg_pointer_create( - const_cast(GRPC_ARG_FAKE_RESOLVER_RESPONSE_GENERATOR), generator, - &response_generator_arg_vtable); + grpc_arg arg; + arg.type = GRPC_ARG_POINTER; + arg.key = (char*)GRPC_ARG_FAKE_RESOLVER_RESPONSE_GENERATOR; + arg.value.pointer.p = generator; + arg.value.pointer.vtable = &response_generator_arg_vtable; + return arg; } FakeResolverResponseGenerator* FakeResolverResponseGenerator::GetFromArgs( const grpc_channel_args* args) { - return grpc_channel_args_get_pointer( - args, GRPC_ARG_FAKE_RESOLVER_RESPONSE_GENERATOR); + const grpc_arg* arg = + grpc_channel_args_find(args, GRPC_ARG_FAKE_RESOLVER_RESPONSE_GENERATOR); + if (arg == nullptr || arg->type != GRPC_ARG_POINTER) return nullptr; + return static_cast(arg->value.pointer.p); } // diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 5db5d1f6c08..f010002ab9c 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -736,8 +736,9 @@ void grpc_get_subchannel_address_arg(const grpc_channel_args* args, } const char* grpc_get_subchannel_address_uri_arg(const grpc_channel_args* args) { - const char* addr_str = - grpc_channel_args_get_string(args, GRPC_ARG_SUBCHANNEL_ADDRESS); + const grpc_arg* addr_arg = + grpc_channel_args_find(args, GRPC_ARG_SUBCHANNEL_ADDRESS); + const char* addr_str = grpc_channel_arg_get_string(addr_arg); GPR_ASSERT(addr_str != nullptr); // Should have been set by LB policy. return addr_str; } diff --git a/src/core/ext/filters/deadline/deadline_filter.cc b/src/core/ext/filters/deadline/deadline_filter.cc index caa547204b9..e0a41a36376 100644 --- a/src/core/ext/filters/deadline/deadline_filter.cc +++ b/src/core/ext/filters/deadline/deadline_filter.cc @@ -358,8 +358,8 @@ const grpc_channel_filter grpc_server_deadline_filter = { }; bool grpc_deadline_checking_enabled(const grpc_channel_args* channel_args) { - return grpc_channel_args_get_bool( - channel_args, GRPC_ARG_ENABLE_DEADLINE_CHECKS, + return grpc_channel_arg_get_bool( + grpc_channel_args_find(channel_args, GRPC_ARG_ENABLE_DEADLINE_CHECKS), !grpc_channel_args_want_minimal_stack(channel_args)); } 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 fe7f2ba705e..ae94ce47b9e 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -23,7 +23,6 @@ #include #include #include "src/core/ext/filters/http/client/http_client_filter.h" -#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/manual_constructor.h" #include "src/core/lib/profiling/timers.h" @@ -436,43 +435,64 @@ static void destroy_call_elem(grpc_call_element* elem, const grpc_call_final_info* final_info, grpc_closure* ignored) {} -static grpc_mdelem scheme_from_args(const grpc_channel_args* channel_args) { +static grpc_mdelem scheme_from_args(const grpc_channel_args* args) { + unsigned i; + size_t j; grpc_mdelem valid_schemes[] = {GRPC_MDELEM_SCHEME_HTTP, GRPC_MDELEM_SCHEME_HTTPS}; - char* scheme = - grpc_channel_args_get_string(channel_args, GRPC_ARG_HTTP2_SCHEME); - if (scheme != nullptr) { - for (size_t i = 0; i < GPR_ARRAY_SIZE(valid_schemes); i++) { - if (0 == grpc_slice_str_cmp(GRPC_MDVALUE(valid_schemes[i]), scheme)) { - return valid_schemes[i]; + if (args != nullptr) { + for (i = 0; i < args->num_args; ++i) { + if (args->args[i].type == GRPC_ARG_STRING && + strcmp(args->args[i].key, GRPC_ARG_HTTP2_SCHEME) == 0) { + for (j = 0; j < GPR_ARRAY_SIZE(valid_schemes); j++) { + if (0 == grpc_slice_str_cmp(GRPC_MDVALUE(valid_schemes[j]), + args->args[i].value.string)) { + return valid_schemes[j]; + } + } } } } return GRPC_MDELEM_SCHEME_HTTP; } -static size_t max_payload_size_from_args( - const grpc_channel_args* channel_args) { - return grpc_channel_args_get_integer( - channel_args, GRPC_ARG_MAX_PAYLOAD_SIZE_FOR_GET, - {kMaxPayloadSizeForGet, 0, kMaxPayloadSizeForGet}); +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 (args->args[i].type != GRPC_ARG_INTEGER) { + gpr_log(GPR_ERROR, "%s: must be an integer", + GRPC_ARG_MAX_PAYLOAD_SIZE_FOR_GET); + } else { + return static_cast(args->args[i].value.integer); + } + } + } + } + return kMaxPayloadSizeForGet; } static grpc_slice user_agent_from_args(const grpc_channel_args* args, const char* transport_name) { gpr_strvec v; + size_t i; int is_first = 1; char* tmp; grpc_slice result; gpr_strvec_init(&v); - char* user_agent_str = - grpc_channel_args_get_string(args, GRPC_ARG_PRIMARY_USER_AGENT_STRING); - if (user_agent_str != nullptr) { - if (!is_first) gpr_strvec_add(&v, gpr_strdup(" ")); - is_first = 0; - gpr_strvec_add(&v, gpr_strdup(user_agent_str)); + for (i = 0; args && i < args->num_args; i++) { + if (0 == strcmp(args->args[i].key, GRPC_ARG_PRIMARY_USER_AGENT_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 { + if (!is_first) gpr_strvec_add(&v, gpr_strdup(" ")); + is_first = 0; + gpr_strvec_add(&v, gpr_strdup(args->args[i].value.string)); + } + } } gpr_asprintf(&tmp, "%sgrpc-c/%s (%s; %s; %s)", is_first ? "" : " ", @@ -481,11 +501,17 @@ static grpc_slice user_agent_from_args(const grpc_channel_args* args, is_first = 0; gpr_strvec_add(&v, tmp); - user_agent_str = - grpc_channel_args_get_string(args, GRPC_ARG_SECONDARY_USER_AGENT_STRING); - if (user_agent_str != nullptr) { - gpr_strvec_add(&v, gpr_strdup(" ")); - gpr_strvec_add(&v, gpr_strdup(user_agent_str)); + for (i = 0; args && i < args->num_args; i++) { + if (0 == strcmp(args->args[i].key, GRPC_ARG_SECONDARY_USER_AGENT_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 { + if (!is_first) gpr_strvec_add(&v, gpr_strdup(" ")); + is_first = 0; + gpr_strvec_add(&v, gpr_strdup(args->args[i].value.string)); + } + } } tmp = gpr_strvec_flatten(&v, nullptr); diff --git a/src/core/ext/filters/http/http_filters_plugin.cc b/src/core/ext/filters/http/http_filters_plugin.cc index d3cbe50016e..f03fa0141df 100644 --- a/src/core/ext/filters/http/http_filters_plugin.cc +++ b/src/core/ext/filters/http/http_filters_plugin.cc @@ -48,8 +48,8 @@ static bool maybe_add_optional_filter(grpc_channel_stack_builder* builder, optional_filter* filtarg = static_cast(arg); const grpc_channel_args* channel_args = grpc_channel_stack_builder_get_channel_arguments(builder); - bool enable = grpc_channel_args_get_bool( - channel_args, filtarg->control_channel_arg, + bool enable = grpc_channel_arg_get_bool( + grpc_channel_args_find(channel_args, filtarg->control_channel_arg), !grpc_channel_args_want_minimal_stack(channel_args)); return enable ? grpc_channel_stack_builder_prepend_filter( builder, filtarg->filter, nullptr, nullptr) diff --git a/src/core/ext/filters/load_reporting/server_load_reporting_plugin.cc b/src/core/ext/filters/load_reporting/server_load_reporting_plugin.cc index 73bcf242d89..667c0c56efd 100644 --- a/src/core/ext/filters/load_reporting/server_load_reporting_plugin.cc +++ b/src/core/ext/filters/load_reporting/server_load_reporting_plugin.cc @@ -33,7 +33,8 @@ #include "src/core/lib/surface/channel_init.h" static bool is_load_reporting_enabled(const grpc_channel_args* a) { - return grpc_channel_args_get_bool(a, GRPC_ARG_ENABLE_LOAD_REPORTING, false); + return grpc_channel_arg_get_bool( + grpc_channel_args_find(a, GRPC_ARG_ENABLE_LOAD_REPORTING), false); } static bool maybe_add_server_load_reporting_filter( diff --git a/src/core/ext/filters/max_age/max_age_filter.cc b/src/core/ext/filters/max_age/max_age_filter.cc index bbe4998f000..1fe8288bd0a 100644 --- a/src/core/ext/filters/max_age/max_age_filter.cc +++ b/src/core/ext/filters/max_age/max_age_filter.cc @@ -519,12 +519,13 @@ static bool maybe_add_max_age_filter(grpc_channel_stack_builder* builder, void* arg) { const grpc_channel_args* channel_args = grpc_channel_stack_builder_get_channel_arguments(builder); - bool enable = grpc_channel_args_get_integer( - channel_args, GRPC_ARG_MAX_CONNECTION_AGE_MS, - MAX_CONNECTION_AGE_INTEGER_OPTIONS) != INT_MAX || - grpc_channel_args_get_integer( - channel_args, GRPC_ARG_MAX_CONNECTION_IDLE_MS, - MAX_CONNECTION_IDLE_INTEGER_OPTIONS) != INT_MAX; + bool enable = + grpc_channel_arg_get_integer( + grpc_channel_args_find(channel_args, GRPC_ARG_MAX_CONNECTION_AGE_MS), + MAX_CONNECTION_AGE_INTEGER_OPTIONS) != INT_MAX || + grpc_channel_arg_get_integer( + grpc_channel_args_find(channel_args, GRPC_ARG_MAX_CONNECTION_IDLE_MS), + MAX_CONNECTION_IDLE_INTEGER_OPTIONS) != INT_MAX; if (enable) { return grpc_channel_stack_builder_prepend_filter( builder, &grpc_max_age_filter, nullptr, nullptr); 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 9815c931458..c7fc3f2e627 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -254,8 +254,9 @@ static grpc_error* init_channel_elem(grpc_channel_element* elem, channel_data* chand = static_cast(elem->channel_data); chand->limits = get_message_size_limits(args->channel_args); // Get method config table from channel args. - const char* service_config_str = - grpc_channel_args_get_string(args->channel_args, GRPC_ARG_SERVICE_CONFIG); + const grpc_arg* channel_arg = + grpc_channel_args_find(args->channel_args, GRPC_ARG_SERVICE_CONFIG); + const char* service_config_str = grpc_channel_arg_get_string(channel_arg); if (service_config_str != nullptr) { grpc_core::UniquePtr service_config = grpc_core::ServiceConfig::Create(service_config_str); diff --git a/src/core/ext/transport/chttp2/client/authority.cc b/src/core/ext/transport/chttp2/client/authority.cc index e3ef47c1997..bad3153b013 100644 --- a/src/core/ext/transport/chttp2/client/authority.cc +++ b/src/core/ext/transport/chttp2/client/authority.cc @@ -28,8 +28,9 @@ grpc_channel_args* grpc_default_authority_add_if_not_present( size_t num_new_args = 0; grpc_core::UniquePtr default_authority; if (!has_default_authority) { - const char* server_uri_str = - grpc_channel_args_get_string(args, GRPC_ARG_SERVER_URI); + const grpc_arg* server_uri_arg = + grpc_channel_args_find(args, GRPC_ARG_SERVER_URI); + const char* server_uri_str = grpc_channel_arg_get_string(server_uri_arg); GPR_ASSERT(server_uri_str != nullptr); default_authority = grpc_core::ResolverRegistry::GetDefaultAuthority(server_uri_str); 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 f49bd27b6c3..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 @@ -64,8 +64,9 @@ static grpc_subchannel_args* get_secure_naming_subchannel_args( return nullptr; } // To which address are we connecting? By default, use the server URI. - const char* server_uri_str = - grpc_channel_args_get_string(args->args, GRPC_ARG_SERVER_URI); + const grpc_arg* server_uri_arg = + grpc_channel_args_find(args->args, GRPC_ARG_SERVER_URI); + const char* server_uri_str = grpc_channel_arg_get_string(server_uri_arg); GPR_ASSERT(server_uri_str != nullptr); grpc_uri* server_uri = grpc_uri_parse(server_uri_str, true /* supress errors */); diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 1a924e66e61..a8090d18a65 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -447,19 +447,20 @@ 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)) { - char* opt_target_str = - grpc_channel_arg_get_string(&channel_args->args[i]); - if (opt_target_str == nullptr) { - gpr_log(GPR_ERROR, "null/missing value opt target, assuming 'blend'"); - } else if (0 == strcmp(opt_target_str, "blend")) { + 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")) { t->opt_target = GRPC_CHTTP2_OPTIMIZE_FOR_LATENCY; - } else if (0 == strcmp(opt_target_str, "latency")) { + } else if (0 == strcmp(channel_args->args[i].value.string, "latency")) { t->opt_target = GRPC_CHTTP2_OPTIMIZE_FOR_LATENCY; - } else if (0 == strcmp(opt_target_str, "throughput")) { + } else if (0 == + strcmp(channel_args->args[i].value.string, "throughput")) { t->opt_target = GRPC_CHTTP2_OPTIMIZE_FOR_THROUGHPUT; } else { gpr_log(GPR_ERROR, "%s value '%s' unknown, assuming 'blend'", - GRPC_ARG_OPTIMIZATION_TARGET, opt_target_str); + GRPC_ARG_OPTIMIZATION_TARGET, + channel_args->args[i].value.string); } } else { static const struct { diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index 5d614c1d057..420c2d13e1d 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -1450,8 +1450,20 @@ grpc_transport* grpc_create_cronet_transport(void* engine, const char* target, } strcpy(ct->host, target); - ct->use_packet_coalescing = grpc_channel_args_get_bool( - args, GRPC_ARG_USE_CRONET_PACKET_COALESCING, true); + ct->use_packet_coalescing = true; + if (args) { + for (size_t i = 0; i < args->num_args; i++) { + if (0 == + strcmp(args->args[i].key, GRPC_ARG_USE_CRONET_PACKET_COALESCING)) { + if (GPR_UNLIKELY(args->args[i].type != GRPC_ARG_INTEGER)) { + gpr_log(GPR_ERROR, "%s ignored: it must be an integer", + GRPC_ARG_USE_CRONET_PACKET_COALESCING); + } else { + ct->use_packet_coalescing = (args->args[i].value.integer != 0); + } + } + } + } return &ct->base; diff --git a/src/core/ext/transport/inproc/inproc_transport.cc b/src/core/ext/transport/inproc/inproc_transport.cc index 6e5aa5a46bc..2c3bff5c1e5 100644 --- a/src/core/ext/transport/inproc/inproc_transport.cc +++ b/src/core/ext/transport/inproc/inproc_transport.cc @@ -1204,9 +1204,10 @@ grpc_channel* grpc_inproc_channel_create(grpc_server* server, // Add a default authority channel argument for the client - grpc_arg default_authority_arg = grpc_channel_arg_string_create( - const_cast(GRPC_ARG_DEFAULT_AUTHORITY), - const_cast("inproc.authority")); + grpc_arg default_authority_arg; + default_authority_arg.type = GRPC_ARG_STRING; + default_authority_arg.key = (char*)GRPC_ARG_DEFAULT_AUTHORITY; + default_authority_arg.value.string = (char*)"inproc.authority"; grpc_channel_args* client_args = grpc_channel_args_copy_and_add(args, &default_authority_arg, 1); diff --git a/src/core/lib/channel/channel_args.h b/src/core/lib/channel/channel_args.h index 0c4f56a6889..5ff303a9dc6 100644 --- a/src/core/lib/channel/channel_args.h +++ b/src/core/lib/channel/channel_args.h @@ -23,7 +23,6 @@ #include #include -#include #include "src/core/lib/iomgr/socket_mutator.h" // Channel args are intentionally immutable, to avoid the need for locking. @@ -111,49 +110,13 @@ typedef struct grpc_integer_options { /** Returns the value of \a arg, subject to the contraints in \a options. */ int grpc_channel_arg_get_integer(const grpc_arg* arg, const grpc_integer_options options); -/** convinience helper for the above that finds the arg first. */ -inline int grpc_channel_args_get_integer(const grpc_channel_args* args, - const char* name, - const grpc_integer_options options) { - return grpc_channel_arg_get_integer(grpc_channel_args_find(args, name), - options); -} /** Returns the value of \a arg if \a arg is of type GRPC_ARG_STRING. Otherwise, emits a warning log, and returns nullptr. If arg is nullptr, returns nullptr, and does not emit a warning. */ char* grpc_channel_arg_get_string(const grpc_arg* arg); -/** convinience helper for the above that finds the arg first. */ -inline char* grpc_channel_args_get_string(const grpc_channel_args* args, - const char* name) { - return grpc_channel_arg_get_string(grpc_channel_args_find(args, name)); -} - -/** Returns the value of \a arg if \a arg is of type GRPC_ARG_POINTER - Otherwise, emits a warning log, and returns nullptr. - If arg is nullptr, returns nullptr, and does not emit a warning. */ -template -inline Type* grpc_channel_arg_get_pointer(const grpc_arg* arg) { - if (arg == nullptr) return nullptr; - if (arg->type != GRPC_ARG_POINTER) { - gpr_log(GPR_ERROR, "%s ignored: it must be an pointer", arg->key); - return nullptr; - } - return static_cast(arg->value.pointer.p); -} -/** convinience helper for the above that finds the arg first. */ -template -inline Type* grpc_channel_args_get_pointer(const grpc_channel_args* args, - const char* name) { - return grpc_channel_arg_get_pointer(grpc_channel_args_find(args, name)); -} bool grpc_channel_arg_get_bool(const grpc_arg* arg, bool default_value); -inline bool grpc_channel_args_get_bool(const grpc_channel_args* args, - const char* name, bool default_value) { - return grpc_channel_arg_get_bool(grpc_channel_args_find(args, name), - default_value); -} // Helpers for creating channel args. grpc_arg grpc_channel_arg_string_create(char* name, char* value); diff --git a/src/core/lib/iomgr/resource_quota.cc b/src/core/lib/iomgr/resource_quota.cc index 8a308f71bb0..539bc120cec 100644 --- a/src/core/lib/iomgr/resource_quota.cc +++ b/src/core/lib/iomgr/resource_quota.cc @@ -30,7 +30,6 @@ #include #include -#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gpr/useful.h" #include "src/core/lib/iomgr/combiner.h" @@ -671,9 +670,18 @@ size_t grpc_resource_quota_peek_size(grpc_resource_quota* resource_quota) { grpc_resource_quota* grpc_resource_quota_from_channel_args( const grpc_channel_args* channel_args) { - grpc_resource_quota* rq = grpc_channel_args_get_pointer( - channel_args, GRPC_ARG_RESOURCE_QUOTA); - return rq == nullptr ? grpc_resource_quota_create(nullptr) : rq; + for (size_t i = 0; i < channel_args->num_args; i++) { + if (0 == strcmp(channel_args->args[i].key, GRPC_ARG_RESOURCE_QUOTA)) { + if (channel_args->args[i].type == GRPC_ARG_POINTER) { + return grpc_resource_quota_ref_internal( + static_cast( + channel_args->args[i].value.pointer.p)); + } else { + gpr_log(GPR_DEBUG, GRPC_ARG_RESOURCE_QUOTA " should be a pointer"); + } + } + } + return grpc_resource_quota_create(nullptr); } static void* rq_copy(void* rq) { diff --git a/src/core/lib/iomgr/tcp_client_posix.cc b/src/core/lib/iomgr/tcp_client_posix.cc index 30015fc2033..296ee74311a 100644 --- a/src/core/lib/iomgr/tcp_client_posix.cc +++ b/src/core/lib/iomgr/tcp_client_posix.cc @@ -66,7 +66,6 @@ typedef struct { static grpc_error* prepare_socket(const grpc_resolved_address* addr, int fd, const grpc_channel_args* channel_args) { grpc_error* err = GRPC_ERROR_NONE; - grpc_socket_mutator* mutator = nullptr; GPR_ASSERT(fd >= 0); @@ -80,11 +79,16 @@ static grpc_error* prepare_socket(const grpc_resolved_address* addr, int fd, } err = grpc_set_socket_no_sigpipe_if_possible(fd); if (err != GRPC_ERROR_NONE) goto error; - mutator = grpc_channel_args_get_pointer( - channel_args, GRPC_ARG_SOCKET_MUTATOR); - if (mutator != nullptr) { - err = grpc_set_socket_with_mutator(fd, mutator); - if (err != GRPC_ERROR_NONE) goto error; + if (channel_args) { + for (size_t i = 0; i < channel_args->num_args; i++) { + if (0 == strcmp(channel_args->args[i].key, GRPC_ARG_SOCKET_MUTATOR)) { + GPR_ASSERT(channel_args->args[i].type == GRPC_ARG_POINTER); + grpc_socket_mutator* mutator = static_cast( + channel_args->args[i].value.pointer.p); + err = grpc_set_socket_with_mutator(fd, mutator); + if (err != GRPC_ERROR_NONE) goto error; + } + } } goto done; diff --git a/src/core/lib/iomgr/tcp_server_custom.cc b/src/core/lib/iomgr/tcp_server_custom.cc index a3496de336e..019b354473b 100644 --- a/src/core/lib/iomgr/tcp_server_custom.cc +++ b/src/core/lib/iomgr/tcp_server_custom.cc @@ -26,7 +26,6 @@ #include #include -#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/iomgr/iomgr_custom.h" @@ -81,9 +80,21 @@ static grpc_error* tcp_server_create(grpc_closure* shutdown_complete, const grpc_channel_args* args, grpc_tcp_server** server) { grpc_tcp_server* s = (grpc_tcp_server*)gpr_malloc(sizeof(grpc_tcp_server)); - grpc_resource_quota* rq = grpc_channel_args_get_pointer( - args, GRPC_ARG_RESOURCE_QUOTA); - s->resource_quota = rq == nullptr ? grpc_resource_quota_create(nullptr) : rq; + s->resource_quota = grpc_resource_quota_create(nullptr); + for (size_t i = 0; i < (args == nullptr ? 0 : args->num_args); i++) { + if (0 == strcmp(GRPC_ARG_RESOURCE_QUOTA, args->args[i].key)) { + if (args->args[i].type == GRPC_ARG_POINTER) { + grpc_resource_quota_unref_internal(s->resource_quota); + s->resource_quota = grpc_resource_quota_ref_internal( + (grpc_resource_quota*)args->args[i].value.pointer.p); + } else { + grpc_resource_quota_unref_internal(s->resource_quota); + gpr_free(s); + return GRPC_ERROR_CREATE_FROM_STATIC_STRING( + GRPC_ARG_RESOURCE_QUOTA " must be a pointer to a buffer pool"); + } + } + } gpr_ref_init(&s->refs, 1); s->on_accept_cb = nullptr; s->on_accept_cb_arg = nullptr; diff --git a/src/core/lib/iomgr/tcp_server_posix.cc b/src/core/lib/iomgr/tcp_server_posix.cc index 0a9b4bfd5fa..8ddf684feac 100644 --- a/src/core/lib/iomgr/tcp_server_posix.cc +++ b/src/core/lib/iomgr/tcp_server_posix.cc @@ -64,11 +64,22 @@ static grpc_error* tcp_server_create(grpc_closure* shutdown_complete, s->expand_wildcard_addrs = false; for (size_t i = 0; i < (args == nullptr ? 0 : args->num_args); i++) { if (0 == strcmp(GRPC_ARG_ALLOW_REUSEPORT, args->args[i].key)) { - s->so_reuseport = grpc_channel_arg_get_bool( - &args->args[i], grpc_is_socket_reuse_port_supported()); + if (args->args[i].type == GRPC_ARG_INTEGER) { + s->so_reuseport = grpc_is_socket_reuse_port_supported() && + (args->args[i].value.integer != 0); + } else { + gpr_free(s); + return GRPC_ERROR_CREATE_FROM_STATIC_STRING(GRPC_ARG_ALLOW_REUSEPORT + " must be an integer"); + } } else if (0 == strcmp(GRPC_ARG_EXPAND_WILDCARD_ADDRS, args->args[i].key)) { - s->expand_wildcard_addrs = - grpc_channel_arg_get_bool(&args->args[i], false); + if (args->args[i].type == GRPC_ARG_INTEGER) { + s->expand_wildcard_addrs = (args->args[i].value.integer != 0); + } else { + gpr_free(s); + return GRPC_ERROR_CREATE_FROM_STATIC_STRING( + GRPC_ARG_EXPAND_WILDCARD_ADDRS " must be an integer"); + } } } gpr_ref_init(&s->refs, 1); diff --git a/src/core/lib/iomgr/tcp_server_utils_posix_common.cc b/src/core/lib/iomgr/tcp_server_utils_posix_common.cc index 8c254d587b3..b9f81455729 100644 --- a/src/core/lib/iomgr/tcp_server_utils_posix_common.cc +++ b/src/core/lib/iomgr/tcp_server_utils_posix_common.cc @@ -34,7 +34,6 @@ #include #include -#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/iomgr/sockaddr_utils.h" @@ -150,7 +149,6 @@ grpc_error* grpc_tcp_server_prepare_socket(grpc_tcp_server* s, int fd, bool so_reuseport, int* port) { grpc_resolved_address sockname_temp; grpc_error* err = GRPC_ERROR_NONE; - grpc_socket_mutator* mutator = nullptr; GPR_ASSERT(fd >= 0); @@ -171,11 +169,17 @@ grpc_error* grpc_tcp_server_prepare_socket(grpc_tcp_server* s, int fd, } err = grpc_set_socket_no_sigpipe_if_possible(fd); if (err != GRPC_ERROR_NONE) goto error; - mutator = grpc_channel_args_get_pointer( - s->channel_args, GRPC_ARG_SOCKET_MUTATOR); - if (mutator != nullptr) { - err = grpc_set_socket_with_mutator(fd, mutator); - if (err != GRPC_ERROR_NONE) goto error; + + if (s->channel_args) { + for (size_t i = 0; i < s->channel_args->num_args; i++) { + if (0 == strcmp(s->channel_args->args[i].key, GRPC_ARG_SOCKET_MUTATOR)) { + GPR_ASSERT(s->channel_args->args[i].type == GRPC_ARG_POINTER); + grpc_socket_mutator* mutator = static_cast( + s->channel_args->args[i].value.pointer.p); + err = grpc_set_socket_with_mutator(fd, mutator); + if (err != GRPC_ERROR_NONE) goto error; + } + } } if (bind(fd, reinterpret_cast(const_cast(addr->addr)), diff --git a/src/core/lib/iomgr/udp_server.cc b/src/core/lib/iomgr/udp_server.cc index 99af977da30..bdb2d0e7644 100644 --- a/src/core/lib/iomgr/udp_server.cc +++ b/src/core/lib/iomgr/udp_server.cc @@ -197,8 +197,14 @@ struct grpc_udp_server { }; static grpc_socket_factory* get_socket_factory(const grpc_channel_args* args) { - return grpc_channel_args_get_pointer( - args, GRPC_ARG_SOCKET_FACTORY); + if (args) { + const grpc_arg* arg = grpc_channel_args_find(args, GRPC_ARG_SOCKET_FACTORY); + if (arg) { + GPR_ASSERT(arg->type == GRPC_ARG_POINTER); + return static_cast(arg->value.pointer.p); + } + } + return nullptr; } grpc_udp_server* grpc_udp_server_create(const grpc_channel_args* args) { diff --git a/src/core/lib/security/context/security_context.cc b/src/core/lib/security/context/security_context.cc index 1f93416b232..14051a3f008 100644 --- a/src/core/lib/security/context/security_context.cc +++ b/src/core/lib/security/context/security_context.cc @@ -326,8 +326,23 @@ grpc_arg grpc_auth_context_to_arg(grpc_auth_context* p) { &auth_context_pointer_vtable); } +grpc_auth_context* grpc_auth_context_from_arg(const grpc_arg* arg) { + if (strcmp(arg->key, GRPC_AUTH_CONTEXT_ARG) != 0) return nullptr; + if (arg->type != GRPC_ARG_POINTER) { + gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type, + GRPC_AUTH_CONTEXT_ARG); + return nullptr; + } + return static_cast(arg->value.pointer.p); +} + grpc_auth_context* grpc_find_auth_context_in_args( - const grpc_channel_args* channel_args) { - return grpc_channel_args_get_pointer( - channel_args, GRPC_AUTH_CONTEXT_ARG); + const grpc_channel_args* args) { + size_t i; + if (args == nullptr) return nullptr; + for (i = 0; i < args->num_args; i++) { + grpc_auth_context* p = grpc_auth_context_from_arg(&args->args[i]); + if (p != nullptr) return p; + } + return nullptr; } diff --git a/src/core/lib/security/context/security_context.h b/src/core/lib/security/context/security_context.h index 2f73a5482cd..e782e4f28fe 100644 --- a/src/core/lib/security/context/security_context.h +++ b/src/core/lib/security/context/security_context.h @@ -108,6 +108,7 @@ void grpc_server_security_context_destroy(void* ctx); #define GRPC_AUTH_CONTEXT_ARG "grpc.auth_context" grpc_arg grpc_auth_context_to_arg(grpc_auth_context* c); +grpc_auth_context* grpc_auth_context_from_arg(const grpc_arg* arg); grpc_auth_context* grpc_find_auth_context_in_args( const grpc_channel_args* args); diff --git a/src/core/lib/security/credentials/credentials.cc b/src/core/lib/security/credentials/credentials.cc index edeea29327b..c43cb440ebf 100644 --- a/src/core/lib/security/credentials/credentials.cc +++ b/src/core/lib/security/credentials/credentials.cc @@ -168,10 +168,27 @@ grpc_arg grpc_channel_credentials_to_arg( &credentials_pointer_vtable); } +grpc_channel_credentials* grpc_channel_credentials_from_arg( + const grpc_arg* arg) { + if (strcmp(arg->key, GRPC_ARG_CHANNEL_CREDENTIALS)) return nullptr; + if (arg->type != GRPC_ARG_POINTER) { + gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type, + GRPC_ARG_CHANNEL_CREDENTIALS); + return nullptr; + } + return static_cast(arg->value.pointer.p); +} + grpc_channel_credentials* grpc_channel_credentials_find_in_args( - const grpc_channel_args* channel_args) { - return grpc_channel_args_get_pointer( - channel_args, GRPC_ARG_CHANNEL_CREDENTIALS); + const grpc_channel_args* args) { + size_t i; + if (args == nullptr) return nullptr; + for (i = 0; i < args->num_args; i++) { + grpc_channel_credentials* credentials = + grpc_channel_credentials_from_arg(&args->args[i]); + if (credentials != nullptr) return credentials; + } + return nullptr; } grpc_server_credentials* grpc_server_credentials_ref( @@ -246,8 +263,24 @@ grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials* p) { &cred_ptr_vtable); } +grpc_server_credentials* grpc_server_credentials_from_arg(const grpc_arg* arg) { + if (strcmp(arg->key, GRPC_SERVER_CREDENTIALS_ARG) != 0) return nullptr; + if (arg->type != GRPC_ARG_POINTER) { + gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type, + GRPC_SERVER_CREDENTIALS_ARG); + return nullptr; + } + return static_cast(arg->value.pointer.p); +} + grpc_server_credentials* grpc_find_server_credentials_in_args( - const grpc_channel_args* channel_args) { - return grpc_channel_args_get_pointer( - channel_args, GRPC_SERVER_CREDENTIALS_ARG); + const grpc_channel_args* args) { + size_t i; + if (args == nullptr) return nullptr; + for (i = 0; i < args->num_args; i++) { + grpc_server_credentials* p = + grpc_server_credentials_from_arg(&args->args[i]); + if (p != nullptr) return p; + } + return nullptr; } diff --git a/src/core/lib/security/credentials/credentials.h b/src/core/lib/security/credentials/credentials.h index ba380283cc1..b486d25ab2e 100644 --- a/src/core/lib/security/credentials/credentials.h +++ b/src/core/lib/security/credentials/credentials.h @@ -131,6 +131,10 @@ grpc_channel_credentials_duplicate_without_call_credentials( /* Util to encapsulate the channel credentials in a channel arg. */ grpc_arg grpc_channel_credentials_to_arg(grpc_channel_credentials* credentials); +/* Util to get the channel credentials from a channel arg. */ +grpc_channel_credentials* grpc_channel_credentials_from_arg( + const grpc_arg* arg); + /* Util to find the channel credentials from channel args. */ grpc_channel_credentials* grpc_channel_credentials_find_in_args( const grpc_channel_args* args); @@ -223,6 +227,7 @@ void grpc_server_credentials_unref(grpc_server_credentials* creds); #define GRPC_SERVER_CREDENTIALS_ARG "grpc.server_credentials" grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials* c); +grpc_server_credentials* grpc_server_credentials_from_arg(const grpc_arg* arg); grpc_server_credentials* grpc_find_server_credentials_in_args( const grpc_channel_args* args); diff --git a/src/core/lib/security/credentials/fake/fake_credentials.cc b/src/core/lib/security/credentials/fake/fake_credentials.cc index 08321a85c68..858ab6b41bb 100644 --- a/src/core/lib/security/credentials/fake/fake_credentials.cc +++ b/src/core/lib/security/credentials/fake/fake_credentials.cc @@ -84,8 +84,9 @@ grpc_arg grpc_fake_transport_expected_targets_arg(char* expected_targets) { const char* grpc_fake_transport_get_expected_targets( const grpc_channel_args* args) { - return grpc_channel_args_get_string(args, - GRPC_ARG_FAKE_SECURITY_EXPECTED_TARGETS); + const grpc_arg* expected_target_arg = + grpc_channel_args_find(args, GRPC_ARG_FAKE_SECURITY_EXPECTED_TARGETS); + return grpc_channel_arg_get_string(expected_target_arg); } /* -- Metadata-only test credentials. -- */ diff --git a/src/core/lib/security/credentials/google_default/google_default_credentials.cc b/src/core/lib/security/credentials/google_default/google_default_credentials.cc index fa565d4ef8d..38c91757175 100644 --- a/src/core/lib/security/credentials/google_default/google_default_credentials.cc +++ b/src/core/lib/security/credentials/google_default/google_default_credentials.cc @@ -79,10 +79,13 @@ static grpc_security_status google_default_create_security_connector( grpc_channel_security_connector** sc, grpc_channel_args** new_args) { grpc_google_default_channel_credentials* c = reinterpret_cast(creds); - bool is_grpclb_load_balancer = grpc_channel_args_get_bool( - args, GRPC_ARG_ADDRESS_IS_GRPCLB_LOAD_BALANCER, false); - bool is_backend_from_grpclb_load_balancer = grpc_channel_args_get_bool( - args, GRPC_ARG_ADDRESS_IS_BACKEND_FROM_GRPCLB_LOAD_BALANCER, false); + bool is_grpclb_load_balancer = grpc_channel_arg_get_bool( + grpc_channel_args_find(args, GRPC_ARG_ADDRESS_IS_GRPCLB_LOAD_BALANCER), + false); + bool is_backend_from_grpclb_load_balancer = grpc_channel_arg_get_bool( + grpc_channel_args_find( + args, GRPC_ARG_ADDRESS_IS_BACKEND_FROM_GRPCLB_LOAD_BALANCER), + false); bool use_alts = is_grpclb_load_balancer || is_backend_from_grpclb_load_balancer; grpc_security_status status = GRPC_SECURITY_ERROR; diff --git a/src/core/lib/security/credentials/ssl/ssl_credentials.cc b/src/core/lib/security/credentials/ssl/ssl_credentials.cc index 13dae19b4b0..2b6377d3ec6 100644 --- a/src/core/lib/security/credentials/ssl/ssl_credentials.cc +++ b/src/core/lib/security/credentials/ssl/ssl_credentials.cc @@ -60,12 +60,14 @@ static grpc_security_status ssl_create_security_connector( tsi_ssl_session_cache* ssl_session_cache = nullptr; for (size_t i = 0; args && i < args->num_args; i++) { grpc_arg* arg = &args->args[i]; - if (strcmp(arg->key, GRPC_SSL_TARGET_NAME_OVERRIDE_ARG) == 0) { - overridden_target_name = grpc_channel_arg_get_string(arg); + if (strcmp(arg->key, GRPC_SSL_TARGET_NAME_OVERRIDE_ARG) == 0 && + arg->type == GRPC_ARG_STRING) { + overridden_target_name = arg->value.string; } - if (strcmp(arg->key, GRPC_SSL_SESSION_CACHE_ARG) == 0) { + if (strcmp(arg->key, GRPC_SSL_SESSION_CACHE_ARG) == 0 && + arg->type == GRPC_ARG_POINTER) { ssl_session_cache = - grpc_channel_arg_get_pointer(arg); + static_cast(arg->value.pointer.p); } } status = grpc_ssl_channel_security_connector_create( diff --git a/src/core/lib/security/security_connector/security_connector.cc b/src/core/lib/security/security_connector/security_connector.cc index ea001d453df..b54a7643e4a 100644 --- a/src/core/lib/security/security_connector/security_connector.cc +++ b/src/core/lib/security/security_connector/security_connector.cc @@ -255,10 +255,26 @@ grpc_arg grpc_security_connector_to_arg(grpc_security_connector* sc) { &connector_arg_vtable); } +grpc_security_connector* grpc_security_connector_from_arg(const grpc_arg* arg) { + if (strcmp(arg->key, GRPC_ARG_SECURITY_CONNECTOR)) return nullptr; + if (arg->type != GRPC_ARG_POINTER) { + gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type, + GRPC_ARG_SECURITY_CONNECTOR); + return nullptr; + } + return static_cast(arg->value.pointer.p); +} + grpc_security_connector* grpc_security_connector_find_in_args( - const grpc_channel_args* channel_args) { - return grpc_channel_args_get_pointer( - channel_args, GRPC_ARG_SECURITY_CONNECTOR); + const grpc_channel_args* args) { + size_t i; + if (args == nullptr) return nullptr; + for (i = 0; i < args->num_args; i++) { + grpc_security_connector* sc = + grpc_security_connector_from_arg(&args->args[i]); + if (sc != nullptr) return sc; + } + return nullptr; } static tsi_client_certificate_request_type diff --git a/src/core/lib/security/security_connector/security_connector.h b/src/core/lib/security/security_connector/security_connector.h index 9da66ef01d8..f9723166d02 100644 --- a/src/core/lib/security/security_connector/security_connector.h +++ b/src/core/lib/security/security_connector/security_connector.h @@ -99,6 +99,9 @@ int grpc_security_connector_cmp(grpc_security_connector* sc, /* Util to encapsulate the connector in a channel arg. */ grpc_arg grpc_security_connector_to_arg(grpc_security_connector* sc); +/* Util to get the connector from a channel arg. */ +grpc_security_connector* grpc_security_connector_from_arg(const grpc_arg* arg); + /* Util to find the connector from channel args. */ grpc_security_connector* grpc_security_connector_find_in_args( const grpc_channel_args* args); diff --git a/src/core/lib/security/transport/target_authority_table.cc b/src/core/lib/security/transport/target_authority_table.cc index 467e681a50d..1eeb557f6a4 100644 --- a/src/core/lib/security/transport/target_authority_table.cc +++ b/src/core/lib/security/transport/target_authority_table.cc @@ -59,8 +59,17 @@ grpc_arg CreateTargetAuthorityTableChannelArg(TargetAuthorityTable* table) { TargetAuthorityTable* FindTargetAuthorityTableInArgs( const grpc_channel_args* args) { - return grpc_channel_args_get_pointer( - args, GRPC_ARG_TARGET_AUTHORITY_TABLE); + const grpc_arg* arg = + grpc_channel_args_find(args, GRPC_ARG_TARGET_AUTHORITY_TABLE); + if (arg != nullptr) { + if (arg->type == GRPC_ARG_POINTER) { + return static_cast(arg->value.pointer.p); + } else { + gpr_log(GPR_ERROR, "value of " GRPC_ARG_TARGET_AUTHORITY_TABLE + " channel arg was not pointer type; ignoring"); + } + } + return nullptr; } } // namespace grpc_core diff --git a/test/core/end2end/fixtures/h2_http_proxy.cc b/test/core/end2end/fixtures/h2_http_proxy.cc index c2ac209cf95..0af8a29a152 100644 --- a/test/core/end2end/fixtures/h2_http_proxy.cc +++ b/test/core/end2end/fixtures/h2_http_proxy.cc @@ -69,8 +69,9 @@ void chttp2_init_client_fullstack(grpc_end2end_test_fixture* f, char* proxy_uri; /* If testing for proxy auth, add credentials to proxy uri */ - const char* proxy_auth_str = - grpc_channel_args_get_string(client_args, GRPC_ARG_HTTP_PROXY_AUTH_CREDS); + const grpc_arg* proxy_auth_arg = + grpc_channel_args_find(client_args, GRPC_ARG_HTTP_PROXY_AUTH_CREDS); + const char* proxy_auth_str = grpc_channel_arg_get_string(proxy_auth_arg); if (proxy_auth_str == nullptr) { gpr_asprintf(&proxy_uri, "http://%s", grpc_end2end_http_proxy_get_proxy_name(ffd->proxy)); diff --git a/test/core/end2end/fixtures/http_proxy_fixture.cc b/test/core/end2end/fixtures/http_proxy_fixture.cc index 5caddee09eb..f02fa9d9983 100644 --- a/test/core/end2end/fixtures/http_proxy_fixture.cc +++ b/test/core/end2end/fixtures/http_proxy_fixture.cc @@ -411,8 +411,9 @@ static void on_read_request_done(void* arg, grpc_error* error) { return; } // If proxy auth is being used, check if the header is present and as expected - char* proxy_auth_str = grpc_channel_args_get_string( + const grpc_arg* proxy_auth_arg = grpc_channel_args_find( conn->proxy->channel_args, GRPC_ARG_HTTP_PROXY_AUTH_CREDS); + char* proxy_auth_str = grpc_channel_arg_get_string(proxy_auth_arg); if (proxy_auth_str != nullptr) { bool client_authenticated = false; for (size_t i = 0; i < conn->http_request.hdr_count; i++) { diff --git a/tools/run_tests/sanity/check_channel_arg_usage.py b/tools/run_tests/sanity/check_channel_arg_usage.py deleted file mode 100755 index bb9f9299c19..00000000000 --- a/tools/run_tests/sanity/check_channel_arg_usage.py +++ /dev/null @@ -1,55 +0,0 @@ -#!/usr/bin/env python - -# Copyright 2018 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from __future__ import print_function - -import os -import sys - -os.chdir(os.path.join(os.path.dirname(sys.argv[0]), '../../..')) - -# set of files that are allowed to use the raw GRPC_ARG_* types -_EXCEPTIONS = set([ - 'src/core/lib/channel/channel_args.cc', - 'src/core/lib/channel/channel_args.h', -]) - -_BANNED = set([ - "GRPC_ARG_POINTER", - "GRPC_ARG_STRING", - "GRPC_ARG_INTEGER", -]) - -errors = 0 -num_files = 0 -for root, dirs, files in os.walk('src/core'): - for filename in files: - num_files += 1 - path = os.path.join(root, filename) - if path in _EXCEPTIONS: continue - with open(path) as f: - text = f.read() - for banned in _BANNED: - if banned in text: - print('Illegal use of "%s" in %s' % (banned, path)) - errors += 1 - -assert errors == 0 -# This check comes about from this issue: -# https://github.com/grpc/grpc/issues/15381 -# Basically, a change rendered this script useless and we did not realize it. -# This dumb check ensures that this type of issue doesn't occur again. -assert num_files > 300 # we definitely have more than 300 files