parse_feature_request: make it easier to see feature values

We already take care to parse key/value capabilities like "foo=bar", but the code does not provide a good way of actually finding out what is on the right-hand side of the "=". A server using "parse_feature_request" could accomplish this with some extra parsing. You must skip past the "key" portion manually, check for "=" versus NUL or space, and then find the length by searching for the next space (or NUL). But clients can't even do that, since the "server_supports" interface does not even return the pointer. Instead, let's have our parser share more information by providing a pointer to the value and its length. The "parse_feature_value" function returns a pointer to the feature's value portion, along with the length of the value. If the feature is missing, NULL is returned. If it does not have an "=", then a zero-length value is returned. Similarly, "server_feature_value" behaves in the same way, but always checks the static server_feature_list variable. We can then implement "server_supports" in terms of "server_feature_value". We cannot implement the original "parse_feature_request" in terms of our new function, because it returned a pointer to the beginning of the feature. However, no callers actually cared about the value of the returned pointer, so we can simplify it to a boolean just as we do for "server_supports". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 13, 2012 at 21:59 UTC 9442710801b0b7b9aeefe60408d1835af138cfcc
2 files changed +39 -10
cache.h
+3 -1
@@ -1030,7 +1030,9 @@ struct extra_have_objects {
1030 };
1031 extern struct ref **get_remote_heads(int in, struct ref **list, unsigned int flags, struct extra_have_objects *);
1032 extern int server_supports(const char *feature);
1033 -extern const char *parse_feature_request(const char *features, const char *feature);
1033 +extern int parse_feature_request(const char *features, const char *feature);
1034 +extern const char *server_feature_value(const char *feature, int *len_ret);
1035 +extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret);
1036
1037 extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
1038
connect.c
+36 -9
@@ -99,12 +99,7 @@ struct ref **get_remote_heads(int in, struct ref **list,
99 return list;
100 }
101
102 -int server_supports(const char *feature)
103 -{
104 - return !!parse_feature_request(server_capabilities, feature);
105 -}
106 -
107 -const char *parse_feature_request(const char *feature_list, const char *feature)
102 +const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)
103 {
104 int len;
105
@@ -116,14 +111,46 @@ const char *parse_feature_request(const char *feature_list, const char *feature)
111 const char *found = strstr(feature_list, feature);
112 if (!found)
113 return NULL;
119 - if ((feature_list == found || isspace(found[-1])) &&
120 - (!found[len] || isspace(found[len]) || found[len] == '='))
121 - return found;
114 + if (feature_list == found || isspace(found[-1])) {
115 + const char *value = found + len;
116 + /* feature with no value (e.g., "thin-pack") */
117 + if (!*value || isspace(*value)) {
118 + if (lenp)
119 + *lenp = 0;
120 + return value;
121 + }
122 + /* feature with a value (e.g., "agent=git/1.2.3") */
123 + else if (*value == '=') {
124 + value++;
125 + if (lenp)
126 + *lenp = strcspn(value, " \t\n");
127 + return value;
128 + }
129 + /*
130 + * otherwise we matched a substring of another feature;
131 + * keep looking
132 + */
133 + }
134 feature_list = found + 1;
135 }
136 return NULL;
137 }
138
139 +int parse_feature_request(const char *feature_list, const char *feature)
140 +{
141 + return !!parse_feature_value(feature_list, feature, NULL);
142 +}
143 +
144 +const char *server_feature_value(const char *feature, int *len)
145 +{
146 + return parse_feature_value(server_capabilities, feature, len);
147 +}
148 +
149 +int server_supports(const char *feature)
150 +{
151 + return !!server_feature_value(feature, NULL);
152 +}
153 +
154 enum protocol {
155 PROTO_LOCAL = 1,
156 PROTO_SSH,