On Jul 11, 2013, at 12:26, Junio C Hamano wrote:
"Kyle J. McKay" <mackyle@xxxxxxxxx> writes:
+static int http_option_maxlen[opt_max];
I understand that this is to keep track of the length of the longest
one that has matched (hence the current candidate). The name "maxlen"
captures the "longest" part, but "has matched" is somehow lost.
Can we have a better name here please, or at least a comment to
clarify what this variable keeps track of.
Will do. How about "http_option_max_matched_len"?
+static int http_options_url_match_prefix(const char *url, const
char *url_prefix)
+{
+ /*
+ * url_prefix matches url if url_prefix is an exact match for url
or it
+ * is a prefix of url and the match ends on a path component
boundary.
+ * url_prefix is considered to have an implicit '/' on the end for
+ * matching purposes if it does not already and it is shorter
than url.
+ * the return value is the length of the match in characters
(excluding
+ * any final '/') or 0 for no match.
+ */
+ size_t url_len, url_prefix_len = strlen(url_prefix);
+ if (!url_prefix_len || strncmp(url, url_prefix, url_prefix_len))
+ return 0;
Should "url=http://git.or.xz/git"; match "url_prefix=http://git.or.xz/git/
"?
My initial thought was no. But your example is persuasive and
probably should match. Fix forthcoming.
+ url_len = strlen(url);
+ if (url_len == url_prefix_len || url[url_prefix_len - 1] == '/'
|| url[url_prefix_len] == '/')
+ return url[url_prefix_len - 1] == '/' ? url_prefix_len - 1 :
url_prefix_len;
Overlong lines that are somewhat hard to read.
OK.
static int http_options(const char *var, const char *value, void *cb)
{
- if (!strcmp("http.sslverify", var)) {
+/*
+ * Macro to ignore matches with a match length less than
previously seen
+ * for the same option type and to remember the largest match
length seen so
+ * far for each option type
+ */
+#define CHECK_MATCHLEN(opt) \
+ if (http_option_maxlen[opt] > matchlen) return 0; \
+ else http_option_maxlen[opt] = matchlen
Avoid defining a macro _inside_ a function. Also if you can make it
a static function, that would be much easier to read.
Was modeled after the credential_match() function from credential.c
that defines a CHECK macro inside it and then undefines it at the end
of the function. I will change it to a static function.
+ if (dot) {
+ char *config_url = xmemdupz(key, dot - key);
+ matchlen = http_options_url_match_prefix(url, config_url);
+ free(config_url);
Yikes. http_options_url_match_prefix() could take a counted string
to config_url to avoid this repeated allocation and deallocation,
no?
Again, modeled after the credential_config_callback() function from
credential.c that does this exact thing.
I will update http_options_url_match_prefix to take a size_t.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html