{"thread":{"id":"34405","subject":"[PATCH v2] config: add support for http.<url>.* settings","startedAt":"2013-07-11T16:47:15Z","lastAt":"2013-07-11T21:49:45Z","messageCount":3,"participants":["Kyle J. McKay","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"223084","messageId":"26507a43bd82855050b90f6a2b9d01bd@f74d39fa044aa309eaea14b9f57fe79c","threadId":"34405","inReplyTo":null,"subject":"[PATCH v2] config: add support for http.<url>.* settings","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-07-11T16:47:15Z","receivedAt":"2013-07-11T16:47:15Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"From: \"Kyle J. McKay\" <mackyle@gmail.com>\n\nThe <url> value is considered a match to a url if the <url> value\nis either an exact match or a prefix of the url which ends on a\npath component boundary ('/').  So \"https://example.com/test\" will\nmatch \"https://example.com/test\" and \"https://example.com/test/too\"\nbut not \"https://example.com/testextra\".\n\nLonger matches take precedence over shorter matches with\nenvironment variable settings taking precedence over all.\n\nWith this configuration:\n\n[http]\n        useragent = other-agent\n        noEPSV = true\n[http \"https://example.com\"]\n        useragent = example-agent\n        sslVerify = false\n[http \"https://example.com/path\"]\n        useragent = path-agent\n\nThe \"https://other.example.com/\" url will have useragent\n\"other-agent\" and sslVerify will be on.\n\nThe \"https://example.com/\" url will have useragent\n\"example-agent\" and sslVerify will be off.\n\nThe \"https://example.com/path/sub\" url will have useragent\n\"path-agent\" and sslVerify will be off.\n\nAll three of the examples will have noEPSV enabled.\n\nSigned-off-by: Kyle J. McKay <mackyle@gmail.com>\n---\n\nThe credentials configuration values already support url-specific\nconfiguration items in the form credential.<url>.*.  This patch\nadds similar support for http configuration values.\n\n Documentation/config.txt |  11 ++++\n http.c                   | 140 +++++++++++++++++++++++++++++++++++++++++------\n 2 files changed, 134 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex b4d4887..3731a3a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1531,6 +1531,17 @@ http.useragent::\n \tof common USER_AGENT strings (but not including those like git/1.7.1).\n \tCan be overridden by the 'GIT_HTTP_USER_AGENT' environment variable.\n \n+http.<url>.*::\n+\tAny of the http.* options above can be applied selectively to some urls.\n+\tFor example \"http.https://example.com.useragent\" would set the user\n+\tagent only for https connections to example.com.  The <url> value\n+\tmatches a url if it is an exact match or a prefix of the url matching\n+\tat a '/' boundary.  Longer <url> matches take precedence over shorter\n+\tones with the environment variable settings taking precedence over all.\n+\tNote that <url> must match the url exactly (other than possibly being a\n+\tprefix).  This means any user, password and/or port setting that appears\n+\tin a url must also appear in <url> to have a successful match.\n+\n i18n.commitEncoding::\n \tCharacter encoding the commit messages are stored in; Git itself\n \tdoes not care per se, but this information is necessary e.g. when\ndiff --git a/http.c b/http.c\nindex 2d086ae..68dc789 100644\n--- a/http.c\n+++ b/http.c\n@@ -30,6 +30,34 @@ static CURL *curl_default;\n \n char curl_errorstr[CURL_ERROR_SIZE];\n \n+enum http_option_type {\n+\topt_post_buffer = 0,\n+\topt_min_sessions,\n+#ifdef USE_CURL_MULTI\n+\topt_max_requests,\n+#endif\n+\topt_ssl_verify,\n+\topt_ssl_try,\n+\topt_ssl_cert,\n+#if LIBCURL_VERSION_NUM >= 0x070903\n+\topt_ssl_key,\n+#endif\n+#if LIBCURL_VERSION_NUM >= 0x070908\n+\topt_ssl_capath,\n+#endif\n+\topt_ssl_cainfo,\n+\topt_low_speed,\n+\topt_low_time,\n+\topt_no_epsv,\n+\topt_http_proxy,\n+\topt_cookie_file,\n+\topt_user_agent,\n+\topt_passwd_req,\n+\topt_max\n+};\n+\n+static int http_option_maxlen[opt_max];\n+\n static int curl_ssl_verify = -1;\n static int curl_ssl_try;\n static const char *ssl_cert;\n@@ -141,34 +169,99 @@ static void process_curl_messages(void)\n }\n #endif\n \n+static int http_options_url_match_prefix(const char *url, const char *url_prefix)\n+{\n+\t/*\n+\t * url_prefix matches url if url_prefix is an exact match for url or it\n+\t * is a prefix of url and the match ends on a path component boundary.\n+\t * url_prefix is considered to have an implicit '/' on the end for\n+\t * matching purposes if it does not already and it is shorter than url.\n+\t * the return value is the length of the match in characters (excluding\n+\t * any final '/') or 0 for no match.\n+\t */\n+\tsize_t url_len, url_prefix_len = strlen(url_prefix);\n+\tif (!url_prefix_len || strncmp(url, url_prefix, url_prefix_len))\n+\t\treturn 0;\n+\turl_len = strlen(url);\n+\tif (url_len == url_prefix_len || url[url_prefix_len - 1] == '/' || url[url_prefix_len] == '/')\n+\t\treturn url[url_prefix_len - 1] == '/' ? url_prefix_len - 1 : url_prefix_len;\n+\treturn 0;\n+}\n+\n static int http_options(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(\"http.sslverify\", var)) {\n+/*\n+ * Macro to ignore matches with a match length less than previously seen\n+ * for the same option type and to remember the largest match length seen so\n+ * far for each option type\n+ */\n+#define CHECK_MATCHLEN(opt) \\\n+\tif (http_option_maxlen[opt] > matchlen) return 0; \\\n+\telse http_option_maxlen[opt] = matchlen\n+\n+\tconst char *url = (const char *)cb;\n+\tconst char *key, *dot;\n+\tint matchlen = 0;\n+\n+\tkey = skip_prefix(var, \"http.\");\n+\tif (!key)\n+\t\treturn git_default_config(var, value, cb);\n+\n+\t/*\n+\t * If the part following the leading \"http.\" contains a '.' then check\n+\t * to see if the part between \"http.\" and the last '.' is a match to\n+\t * url.  if it's not then ignore the setting.  Otherwise set key to\n+\t * point to the option which is the final part after the final '.' and\n+\t * use key in subsequent comparisons to determine the option type.\n+\t */\n+\tdot = strrchr(key, '.');\n+\tif (dot) {\n+\t\tchar *config_url = xmemdupz(key, dot - key);\n+\t\tmatchlen = http_options_url_match_prefix(url, config_url);\n+\t\tfree(config_url);\n+\t\tif (!matchlen)\n+\t\t\treturn 0;\n+\t\tkey = dot + 1;\n+\t}\n+\n+\tif (!strcmp(\"sslverify\", key)) {\n+\t\tCHECK_MATCHLEN(opt_ssl_verify);\n \t\tcurl_ssl_verify = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(\"http.sslcert\", var))\n+\tif (!strcmp(\"sslcert\", key)) {\n+\t\tCHECK_MATCHLEN(opt_ssl_cert);\n \t\treturn git_config_string(&ssl_cert, var, value);\n+\t}\n #if LIBCURL_VERSION_NUM >= 0x070903\n-\tif (!strcmp(\"http.sslkey\", var))\n+\tif (!strcmp(\"sslkey\", key)) {\n+\t\tCHECK_MATCHLEN(opt_ssl_key);\n \t\treturn git_config_string(&ssl_key, var, value);\n+\t}\n #endif\n #if LIBCURL_VERSION_NUM >= 0x070908\n-\tif (!strcmp(\"http.sslcapath\", var))\n+\tif (!strcmp(\"sslcapath\", key)) {\n+\t\tCHECK_MATCHLEN(opt_ssl_capath);\n \t\treturn git_config_string(&ssl_capath, var, value);\n+\t}\n #endif\n-\tif (!strcmp(\"http.sslcainfo\", var))\n+\tif (!strcmp(\"sslcainfo\", key)) {\n+\t\tCHECK_MATCHLEN(opt_ssl_cainfo);\n \t\treturn git_config_string(&ssl_cainfo, var, value);\n-\tif (!strcmp(\"http.sslcertpasswordprotected\", var)) {\n+\t}\n+\tif (!strcmp(\"sslcertpasswordprotected\", key)) {\n+\t\tCHECK_MATCHLEN(opt_passwd_req);\n \t\tif (git_config_bool(var, value))\n \t\t\tssl_cert_password_required = 1;\n \t\treturn 0;\n \t}\n-\tif (!strcmp(\"http.ssltry\", var)) {\n+\tif (!strcmp(\"ssltry\", key)) {\n+\t\tCHECK_MATCHLEN(opt_ssl_try);\n \t\tcurl_ssl_try = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(\"http.minsessions\", var)) {\n+\tif (!strcmp(\"minsessions\", key)) {\n+\t\tCHECK_MATCHLEN(opt_min_sessions);\n \t\tmin_curl_sessions = git_config_int(var, value);\n #ifndef USE_CURL_MULTI\n \t\tif (min_curl_sessions > 1)\n@@ -177,42 +270,55 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n #ifdef USE_CURL_MULTI\n-\tif (!strcmp(\"http.maxrequests\", var)) {\n+\tif (!strcmp(\"maxrequests\", key)) {\n+\t\tCHECK_MATCHLEN(opt_max_requests);\n \t\tmax_requests = git_config_int(var, value);\n \t\treturn 0;\n \t}\n #endif\n-\tif (!strcmp(\"http.lowspeedlimit\", var)) {\n+\tif (!strcmp(\"lowspeedlimit\", key)) {\n+\t\tCHECK_MATCHLEN(opt_low_speed);\n \t\tcurl_low_speed_limit = (long)git_config_int(var, value);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(\"http.lowspeedtime\", var)) {\n+\tif (!strcmp(\"lowspeedtime\", key)) {\n+\t\tCHECK_MATCHLEN(opt_low_time);\n \t\tcurl_low_speed_time = (long)git_config_int(var, value);\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(\"http.noepsv\", var)) {\n+\tif (!strcmp(\"noepsv\", key)) {\n+\t\tCHECK_MATCHLEN(opt_no_epsv);\n \t\tcurl_ftp_no_epsv = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(\"http.proxy\", var))\n+\tif (!strcmp(\"proxy\", key)) {\n+\t\tCHECK_MATCHLEN(opt_http_proxy);\n \t\treturn git_config_string(&curl_http_proxy, var, value);\n+\t}\n \n-\tif (!strcmp(\"http.cookiefile\", var))\n+\tif (!strcmp(\"cookiefile\", key)) {\n+\t\tCHECK_MATCHLEN(opt_cookie_file);\n \t\treturn git_config_string(&curl_cookie_file, var, value);\n+\t}\n \n-\tif (!strcmp(\"http.postbuffer\", var)) {\n+\tif (!strcmp(\"postbuffer\", key)) {\n+\t\tCHECK_MATCHLEN(opt_post_buffer);\n \t\thttp_post_buffer = git_config_int(var, value);\n \t\tif (http_post_buffer < LARGE_PACKET_MAX)\n \t\t\thttp_post_buffer = LARGE_PACKET_MAX;\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(\"http.useragent\", var))\n+\tif (!strcmp(\"useragent\", key)) {\n+\t\tCHECK_MATCHLEN(opt_user_agent);\n \t\treturn git_config_string(&user_agent, var, value);\n+\t}\n \n \t/* Fall back on the default ones */\n \treturn git_default_config(var, value, cb);\n+\n+#undef CHECKLEN\n }\n \n static void init_curl_http_auth(CURL *result)\n@@ -344,7 +450,7 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n \n \thttp_is_verbose = 0;\n \n-\tgit_config(http_options, NULL);\n+\tgit_config(http_options, (void *)url);\n \n \tcurl_global_init(CURL_GLOBAL_ALL);\n \n-- \n1.8.3\n"},{"id":"223093","messageId":"7vd2qorj7s.fsf@alter.siamese.dyndns.org","threadId":"34405","inReplyTo":"26507a43bd82855050b90f6a2b9d01bd@f74d39fa044aa309eaea14b9f57fe79c","subject":"Re: [PATCH v2] config: add support for http.<url>.* settings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-11T19:26:47Z","receivedAt":"2013-07-11T19:26:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kyle J. McKay\" <mackyle@gmail.com> writes:\n\n> Longer matches take precedence over shorter matches with\n> environment variable settings taking precedence over all.\n\nOK.\n\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index b4d4887..3731a3a 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1531,6 +1531,17 @@ http.useragent::\n>  \tof common USER_AGENT strings (but not including those like git/1.7.1).\n>  \tCan be overridden by the 'GIT_HTTP_USER_AGENT' environment variable.\n>  \n> +http.<url>.*::\n> +\tAny of the http.* options above can be applied selectively to some urls.\n> +\tFor example \"http.https://example.com.useragent\" would set the user\n> +\tagent only for https connections to example.com.  The <url> value\n> +\tmatches a url if it is an exact match or a prefix of the url matching\n> +\tat a '/' boundary.  Longer <url> matches take precedence over shorter\n> +\tones with the environment variable settings taking precedence over all.\n> +\tNote that <url> must match the url exactly (other than possibly being a\n> +\tprefix).  This means any user, password and/or port setting that appears\n> +\tin a url must also appear in <url> to have a successful match.\n> +\n\nOK.\n\n> diff --git a/http.c b/http.c\n> index 2d086ae..68dc789 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -30,6 +30,34 @@ static CURL *curl_default;\n>  \n>  char curl_errorstr[CURL_ERROR_SIZE];\n>  \n> +enum http_option_type {\n> +\topt_post_buffer = 0,\n> +\topt_min_sessions,\n> +#ifdef USE_CURL_MULTI\n> +\topt_max_requests,\n> +#endif\n> +\topt_ssl_verify,\n> +\topt_ssl_try,\n> +\topt_ssl_cert,\n> +#if LIBCURL_VERSION_NUM >= 0x070903\n> +\topt_ssl_key,\n> +#endif\n> +#if LIBCURL_VERSION_NUM >= 0x070908\n> +\topt_ssl_capath,\n> +#endif\n> +\topt_ssl_cainfo,\n> +\topt_low_speed,\n> +\topt_low_time,\n> +\topt_no_epsv,\n> +\topt_http_proxy,\n> +\topt_cookie_file,\n> +\topt_user_agent,\n> +\topt_passwd_req,\n> +\topt_max\n> +};\n> +\n> +static int http_option_maxlen[opt_max];\n\nI understand that this is to keep track of the length of the longest\none that has matched (hence the current candidate).  The name \"maxlen\"\ncaptures the \"longest\" part, but \"has matched\" is somehow lost.\n\nCan we have a better name here please, or at least a comment to\nclarify what this variable keeps track of.\n\n>  static int curl_ssl_verify = -1;\n>  static int curl_ssl_try;\n>  static const char *ssl_cert;\n> @@ -141,34 +169,99 @@ static void process_curl_messages(void)\n>  }\n>  #endif\n>  \n> +static int http_options_url_match_prefix(const char *url, const char *url_prefix)\n> +{\n> +\t/*\n> +\t * url_prefix matches url if url_prefix is an exact match for url or it\n> +\t * is a prefix of url and the match ends on a path component boundary.\n> +\t * url_prefix is considered to have an implicit '/' on the end for\n> +\t * matching purposes if it does not already and it is shorter than url.\n> +\t * the return value is the length of the match in characters (excluding\n> +\t * any final '/') or 0 for no match.\n> +\t */\n> +\tsize_t url_len, url_prefix_len = strlen(url_prefix);\n> +\tif (!url_prefix_len || strncmp(url, url_prefix, url_prefix_len))\n> +\t\treturn 0;\n\nShould \"url=http://git.or.xz/git\" match \"url_prefix=http://git.or.xz/git/\"?\n\n> +\turl_len = strlen(url);\n> +\tif (url_len == url_prefix_len || url[url_prefix_len - 1] == '/' || url[url_prefix_len] == '/')\n> +\t\treturn url[url_prefix_len - 1] == '/' ? url_prefix_len - 1 : url_prefix_len;\n\nOverlong lines that are somewhat hard to read.\n\n> +\treturn 0;\n> +}\n> +\n>  static int http_options(const char *var, const char *value, void *cb)\n>  {\n> -\tif (!strcmp(\"http.sslverify\", var)) {\n> +/*\n> + * Macro to ignore matches with a match length less than previously seen\n> + * for the same option type and to remember the largest match length seen so\n> + * far for each option type\n> + */\n> +#define CHECK_MATCHLEN(opt) \\\n> +\tif (http_option_maxlen[opt] > matchlen) return 0; \\\n> +\telse http_option_maxlen[opt] = matchlen\n\nAvoid defining a macro _inside_ a function.  Also if you can make it\na static function, that would be much easier to read.\n\n> +\n> +\tconst char *url = (const char *)cb;\n> +\tconst char *key, *dot;\n> +\tint matchlen = 0;\n\n> +\tkey = skip_prefix(var, \"http.\");\n> +\tif (!key)\n> +\t\treturn git_default_config(var, value, cb);\n> +\n> +\t/*\n> +\t * If the part following the leading \"http.\" contains a '.' then check\n> +\t * to see if the part between \"http.\" and the last '.' is a match to\n> +\t * url.  if it's not then ignore the setting.  Otherwise set key to\n> +\t * point to the option which is the final part after the final '.' and\n> +\t * use key in subsequent comparisons to determine the option type.\n> +\t */\n> +\tdot = strrchr(key, '.');\n> +\tif (dot) {\n> +\t\tchar *config_url = xmemdupz(key, dot - key);\n> +\t\tmatchlen = http_options_url_match_prefix(url, config_url);\n> +\t\tfree(config_url);\n\nYikes.  http_options_url_match_prefix() could take a counted string\nto config_url to avoind this repeated allocation and deallocation,\nno?\n\n> +\t\tif (!matchlen)\n> +\t\t\treturn 0;\n> +\t\tkey = dot + 1;\n> +\t}\n> +\n> +\tif (!strcmp(\"sslverify\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_ssl_verify);\n>  \t\tcurl_ssl_verify = git_config_bool(var, value);\n>  \t\treturn 0;\n>  \t}\n> -\tif (!strcmp(\"http.sslcert\", var))\n> +\tif (!strcmp(\"sslcert\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_ssl_cert);\n>  \t\treturn git_config_string(&ssl_cert, var, value);\n> +\t}\n>  #if LIBCURL_VERSION_NUM >= 0x070903\n> -\tif (!strcmp(\"http.sslkey\", var))\n> +\tif (!strcmp(\"sslkey\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_ssl_key);\n>  \t\treturn git_config_string(&ssl_key, var, value);\n> +\t}\n>  #endif\n>  #if LIBCURL_VERSION_NUM >= 0x070908\n> -\tif (!strcmp(\"http.sslcapath\", var))\n> +\tif (!strcmp(\"sslcapath\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_ssl_capath);\n>  \t\treturn git_config_string(&ssl_capath, var, value);\n> +\t}\n>  #endif\n> -\tif (!strcmp(\"http.sslcainfo\", var))\n> +\tif (!strcmp(\"sslcainfo\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_ssl_cainfo);\n>  \t\treturn git_config_string(&ssl_cainfo, var, value);\n> -\tif (!strcmp(\"http.sslcertpasswordprotected\", var)) {\n> +\t}\n> +\tif (!strcmp(\"sslcertpasswordprotected\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_passwd_req);\n>  \t\tif (git_config_bool(var, value))\n>  \t\t\tssl_cert_password_required = 1;\n>  \t\treturn 0;\n>  \t}\n> -\tif (!strcmp(\"http.ssltry\", var)) {\n> +\tif (!strcmp(\"ssltry\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_ssl_try);\n>  \t\tcurl_ssl_try = git_config_bool(var, value);\n>  \t\treturn 0;\n>  \t}\n> -\tif (!strcmp(\"http.minsessions\", var)) {\n> +\tif (!strcmp(\"minsessions\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_min_sessions);\n>  \t\tmin_curl_sessions = git_config_int(var, value);\n>  #ifndef USE_CURL_MULTI\n>  \t\tif (min_curl_sessions > 1)\n> @@ -177,42 +270,55 @@ static int http_options(const char *var, const char *value, void *cb)\n>  \t\treturn 0;\n>  \t}\n>  #ifdef USE_CURL_MULTI\n> -\tif (!strcmp(\"http.maxrequests\", var)) {\n> +\tif (!strcmp(\"maxrequests\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_max_requests);\n>  \t\tmax_requests = git_config_int(var, value);\n>  \t\treturn 0;\n>  \t}\n>  #endif\n> -\tif (!strcmp(\"http.lowspeedlimit\", var)) {\n> +\tif (!strcmp(\"lowspeedlimit\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_low_speed);\n>  \t\tcurl_low_speed_limit = (long)git_config_int(var, value);\n>  \t\treturn 0;\n>  \t}\n> -\tif (!strcmp(\"http.lowspeedtime\", var)) {\n> +\tif (!strcmp(\"lowspeedtime\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_low_time);\n>  \t\tcurl_low_speed_time = (long)git_config_int(var, value);\n>  \t\treturn 0;\n>  \t}\n>  \n> -\tif (!strcmp(\"http.noepsv\", var)) {\n> +\tif (!strcmp(\"noepsv\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_no_epsv);\n>  \t\tcurl_ftp_no_epsv = git_config_bool(var, value);\n>  \t\treturn 0;\n>  \t}\n> -\tif (!strcmp(\"http.proxy\", var))\n> +\tif (!strcmp(\"proxy\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_http_proxy);\n>  \t\treturn git_config_string(&curl_http_proxy, var, value);\n> +\t}\n>  \n> -\tif (!strcmp(\"http.cookiefile\", var))\n> +\tif (!strcmp(\"cookiefile\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_cookie_file);\n>  \t\treturn git_config_string(&curl_cookie_file, var, value);\n> +\t}\n>  \n> -\tif (!strcmp(\"http.postbuffer\", var)) {\n> +\tif (!strcmp(\"postbuffer\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_post_buffer);\n>  \t\thttp_post_buffer = git_config_int(var, value);\n>  \t\tif (http_post_buffer < LARGE_PACKET_MAX)\n>  \t\t\thttp_post_buffer = LARGE_PACKET_MAX;\n>  \t\treturn 0;\n>  \t}\n>  \n> -\tif (!strcmp(\"http.useragent\", var))\n> +\tif (!strcmp(\"useragent\", key)) {\n> +\t\tCHECK_MATCHLEN(opt_user_agent);\n>  \t\treturn git_config_string(&user_agent, var, value);\n> +\t}\n>  \n>  \t/* Fall back on the default ones */\n>  \treturn git_default_config(var, value, cb);\n> +\n> +#undef CHECKLEN\n>  }\n>  \n>  static void init_curl_http_auth(CURL *result)\n> @@ -344,7 +450,7 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n>  \n>  \thttp_is_verbose = 0;\n>  \n> -\tgit_config(http_options, NULL);\n> +\tgit_config(http_options, (void *)url);\n>  \n>  \tcurl_global_init(CURL_GLOBAL_ALL);\n"},{"id":"223097","messageId":"072BFEEA-249D-4971-82D3-E76E6C19A303@gmail.com","threadId":"34405","inReplyTo":"7vd2qorj7s.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] config: add support for http.<url>.* settings","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-07-11T21:49:45Z","receivedAt":"2013-07-11T21:49:45Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Jul 11, 2013, at 12:26, Junio C Hamano wrote:\n> \"Kyle J. McKay\" <mackyle@gmail.com> writes:\n>\n>> +static int http_option_maxlen[opt_max];\n>\n> I understand that this is to keep track of the length of the longest\n> one that has matched (hence the current candidate).  The name \"maxlen\"\n> captures the \"longest\" part, but \"has matched\" is somehow lost.\n>\n> Can we have a better name here please, or at least a comment to\n> clarify what this variable keeps track of.\n\nWill do.  How about \"http_option_max_matched_len\"?\n\n>> +static int http_options_url_match_prefix(const char *url, const  \n>> char *url_prefix)\n>> +{\n>> +\t/*\n>> +\t * url_prefix matches url if url_prefix is an exact match for url  \n>> or it\n>> +\t * is a prefix of url and the match ends on a path component  \n>> boundary.\n>> +\t * url_prefix is considered to have an implicit '/' on the end for\n>> +\t * matching purposes if it does not already and it is shorter  \n>> than url.\n>> +\t * the return value is the length of the match in characters  \n>> (excluding\n>> +\t * any final '/') or 0 for no match.\n>> +\t */\n>> +\tsize_t url_len, url_prefix_len = strlen(url_prefix);\n>> +\tif (!url_prefix_len || strncmp(url, url_prefix, url_prefix_len))\n>> +\t\treturn 0;\n>\n> Should \"url=http://git.or.xz/git\" match \"url_prefix=http://git.or.xz/git/ \n> \"?\n\nMy initial thought was no.  But your example is persuasive and  \nprobably should match.  Fix forthcoming.\n\n>> +\turl_len = strlen(url);\n>> +\tif (url_len == url_prefix_len || url[url_prefix_len - 1] == '/'  \n>> || url[url_prefix_len] == '/')\n>> +\t\treturn url[url_prefix_len - 1] == '/' ? url_prefix_len - 1 :  \n>> url_prefix_len;\n>\n> Overlong lines that are somewhat hard to read.\n\nOK.\n\n>> static int http_options(const char *var, const char *value, void *cb)\n>> {\n>> -\tif (!strcmp(\"http.sslverify\", var)) {\n>> +/*\n>> + * Macro to ignore matches with a match length less than  \n>> previously seen\n>> + * for the same option type and to remember the largest match  \n>> length seen so\n>> + * far for each option type\n>> + */\n>> +#define CHECK_MATCHLEN(opt) \\\n>> +\tif (http_option_maxlen[opt] > matchlen) return 0; \\\n>> +\telse http_option_maxlen[opt] = matchlen\n>\n> Avoid defining a macro _inside_ a function.  Also if you can make it\n> a static function, that would be much easier to read.\n\nWas modeled after the credential_match() function from credential.c  \nthat defines a CHECK macro inside it and then undefines it at the end  \nof the function.  I will change it to a static function.\n\n>> +\tif (dot) {\n>> +\t\tchar *config_url = xmemdupz(key, dot - key);\n>> +\t\tmatchlen = http_options_url_match_prefix(url, config_url);\n>> +\t\tfree(config_url);\n>\n> Yikes.  http_options_url_match_prefix() could take a counted string\n> to config_url to avoid this repeated allocation and deallocation,\n> no?\n\nAgain, modeled after the credential_config_callback() function from  \ncredential.c that does this exact thing.\n\nI will update http_options_url_match_prefix to take a size_t.\n"}]}