git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 04/14] connect: rewrite feature parsing to work on string_list

From
Stefan Beller <sbeller@google.com>
Date
Apr 29, 2016, 23:34 UTC
Message-ID
<1461972887-22100-5-git-send-email-sbeller@google.com>
In-Reply-To
<1461972887-22100-1-git-send-email-sbeller@google.com>

Later on when we introduce the version 2 transport protocol, the capabilities will not be transported in one lone string but each capability will be carried in its own pkt line.

To reuse existing infrastructure we would either need to join the capabilities into a single string again later or refactor the current capability parsing to be using a data structure which fits both versions of the transport protocol. We chose to implement the later.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/receive-pack.c | 15 ++++++---
 connect.c              | 82 +++++++++++++++++++++++---------------------------
 connect.h              |  2 +-
 upload-pack.c          | 13 ++++++--
 4 files changed, 58 insertions(+), 54 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a744437..4e98514 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1429,16 +1429,21 @@ static struct command *read_head_info(struct sha1_array *shallow)
 
 		linelen = strlen(line);
 		if (linelen < len) {
-			const char *feature_list = line + linelen + 1;
-			if (parse_feature_request(feature_list, "report-status"))
+			struct string_list feature_list = STRING_LIST_INIT_DUP;
+			string_list_split(&feature_list,
+					  line + linelen + 1,
+					  ' ', -1);
+			if (parse_feature_request(&feature_list, "report-status"))
 				report_status = 1;
-			if (parse_feature_request(feature_list, "side-band-64k"))
+			if (parse_feature_request(&feature_list, "side-band-64k"))
 				use_sideband = LARGE_PACKET_MAX;
-			if (parse_feature_request(feature_list, "quiet"))
+			if (parse_feature_request(&feature_list, "quiet"))
 				quiet = 1;
 			if (advertise_atomic_push
-			    && parse_feature_request(feature_list, "atomic"))
+			    && parse_feature_request(&feature_list, "atomic"))
 				use_atomic = 1;
+
+			string_list_clear(&feature_list, 1);
 		}
 
 		if (!strcmp(line, "push-cert")) {
diff --git a/connect.c b/connect.c
index c53f3f1..79505fb 100644
--- a/connect.c
+++ b/connect.c
@@ -11,8 +11,8 @@
 #include "sha1-array.h"
 #include "transport.h"
 
-static char *server_capabilities;
-static const char *parse_feature_value(const char *, const char *, int *);
+struct string_list server_capabilities = STRING_LIST_INIT_DUP;
+static const char *parse_feature_value(struct string_list *, const char *, int *);
 
 static int check_ref(const char *name, unsigned int flags)
 {
@@ -81,18 +81,18 @@ reject:
 
 static void annotate_refs_with_symref_info(struct ref *ref)
 {
+	struct string_list_item *item;
 	struct string_list symref = STRING_LIST_INIT_DUP;
-	const char *feature_list = server_capabilities;
 
-	while (feature_list) {
-		int len;
+	for_each_string_list_item(item, &server_capabilities) {
 		const char *val;
 
-		val = parse_feature_value(feature_list, "symref", &len);
-		if (!val)
-			break;
-		parse_one_symref_info(&symref, val, len);
-		feature_list = val + 1;
+		if (skip_prefix(item->string, "symref", &val)) {
+			if (!val)
+				continue;
+			val++; /* skip the = */
+			parse_one_symref_info(&symref, val, strlen(val));
+		}
 	}
 	string_list_sort(&symref);
 
@@ -155,9 +155,12 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 		name = buffer + GIT_SHA1_HEXSZ + 1;
 
 		name_len = strlen(name);
+
 		if (len != name_len + GIT_SHA1_HEXSZ + 1) {
-			free(server_capabilities);
-			server_capabilities = xstrdup(name + name_len + 1);
+			string_list_clear(&server_capabilities, 1);
+			string_list_split(&server_capabilities,
+					  name + name_len + 1,
+					  ' ', -1);
 		}
 
 		if (extra_have && !strcmp(name, ".have")) {
@@ -179,51 +182,40 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 	return list;
 }
 
-static const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)
+static const char *parse_feature_value(struct string_list *feature_list, const char *feature, int *lenp)
 {
-	int len;
-
-	if (!feature_list)
-		return NULL;
-
-	len = strlen(feature);
-	while (*feature_list) {
-		const char *found = strstr(feature_list, feature);
-		if (!found)
-			return NULL;
-		if (feature_list == found || isspace(found[-1])) {
-			const char *value = found + len;
-			/* feature with no value (e.g., "thin-pack") */
-			if (!*value || isspace(*value)) {
-				if (lenp)
-					*lenp = 0;
-				return value;
-			}
-			/* feature with a value (e.g., "agent=git/1.2.3") */
-			else if (*value == '=') {
-				value++;
-				if (lenp)
-					*lenp = strcspn(value, " \t\n");
-				return value;
-			}
-			/*
-			 * otherwise we matched a substring of another feature;
-			 * keep looking
-			 */
+	const char *value;
+	struct string_list_item *item;
+
+	for_each_string_list_item(item, feature_list) {
+		if (!skip_prefix(item->string, feature, &value))
+			continue;
+
+		/* feature with no value (e.g., "thin-pack") */
+		if (!*value) {
+			if (lenp)
+				*lenp = 0;
+			return value;
+		}
+		/* feature with a value (e.g., "agent=git/1.2.3") */
+		else if (*value == '=') {
+			value++;
+			if (lenp)
+				*lenp = strlen(value);
+			return value;
 		}
-		feature_list = found + 1;
 	}
 	return NULL;
 }
 
-int parse_feature_request(const char *feature_list, const char *feature)
+int parse_feature_request(struct string_list *feature_list, const char *feature)
 {
 	return !!parse_feature_value(feature_list, feature, NULL);
 }
 
 const char *server_feature_value(const char *feature, int *len)
 {
-	return parse_feature_value(server_capabilities, feature, len);
+	return parse_feature_value(&server_capabilities, feature, len);
 }
 
 int server_supports(const char *feature)
diff --git a/connect.h b/connect.h
index 01f14cd..eaf21c2 100644
--- a/connect.h
+++ b/connect.h
@@ -9,7 +9,7 @@ extern struct child_process *git_connect(int fd[2], const char *url, const char
 extern int finish_connect(struct child_process *conn);
 extern int git_connection_is_socket(struct child_process *conn);
 extern int server_supports(const char *feature);
-extern int parse_feature_request(const char *features, const char *feature);
+extern int parse_feature_request(struct string_list *, const char *feature);
 extern const char *server_feature_value(const char *feature, int *len_ret);
 extern int url_is_local_not_ssh(const char *url);
 
diff --git a/upload-pack.c b/upload-pack.c
index edfd417..4e69b8e 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -535,7 +535,7 @@ error:
 	}
 }
 
-static void parse_features(const char *features)
+static void parse_features(struct string_list *features)
 {
 	if (parse_feature_request(features, "multi_ack_detailed"))
 		multi_ack = 2;
@@ -567,7 +567,9 @@ static void receive_needs(void)
 	for (;;) {
 		struct object *o;
 		unsigned char sha1_buf[20];
+		struct string_list list = STRING_LIST_INIT_DUP;
 		char *line = packet_read_line(0, NULL);
+
 		reset_timeout();
 		if (!line)
 			break;
@@ -600,7 +602,9 @@ static void receive_needs(void)
 			die("git upload-pack: protocol error, "
 			    "expected to get sha, not '%s'", line);
 
-		parse_features(line + 45);
+		string_list_split(&list, line + 45, ' ', -1);
+		parse_features(&list);
+		string_list_clear(&list, 1);
 
 		o = parse_object(sha1_buf);
 		if (!o)
@@ -840,9 +844,12 @@ static void send_capabilities_version_2(void)
 
 static void receive_capabilities_version_2(void)
 {
+	struct string_list list = STRING_LIST_INIT_NODUP;
 	char *line = packet_read_line(0, NULL);
 	while (line) {
-		parse_features(line);
+		string_list_append(&list, line);
+		parse_features(&list);
+		string_list_clear(&list, 1);
 		line = packet_read_line(0, NULL);
 	}
 }

-- 
2.8.0.32.g71f8beb.dirty
Previous: Junio C HamanoNext: David Turner
Message 12 of 40 in “Protocol v2 patches”
  1. 00/14 Protocol v2 patchesStefan Beller, Apr 29, 2016
  2. 01/14 upload-pack: make client capability parsing code a separate functionStefan Beller, Apr 29, 2016
  3. 02/14 upload-pack.c: Refactor capability advertisingStefan Beller, Apr 29, 2016
  4. David TurnerApr 30, 2016
  5. Junio C HamanoMay 4, 2016
  6. 03/14 upload-pack-2: Implement the version 2 of upload-packStefan Beller, Apr 29, 2016
  7. David TurnerMay 2, 2016
  8. Stefan BellerMay 2, 2016
  9. David TurnerMay 2, 2016
  10. Duy NguyenMay 3, 2016
  11. Junio C HamanoMay 4, 2016
  12. 04/14 connect: rewrite feature parsing to work on string_listStefan Beller, Apr 29, 2016
  13. David TurnerMay 2, 2016
  14. Stefan BellerMay 2, 2016
  15. Junio C HamanoMay 4, 2016
  16. David TurnerMay 17, 2016
  17. 05/14 transport: add infrastructure to support a protocol version numberStefan Beller, Apr 29, 2016
  18. 06/14 remote.h: add get_remote_capabilities, request_capabilitiesStefan Beller, Apr 29, 2016
  19. David TurnerMay 2, 2016
  20. Jeff KingMay 3, 2016
  21. David TurnerMay 3, 2016
  22. Stefan BellerMay 4, 2016
  23. 07/14 fetch-pack: move capability selection out of do_fetch_packStefan Beller, Apr 29, 2016
  24. 08/14 fetch-pack: factor out get_selected_capabilities_listStefan Beller, Apr 29, 2016
  25. David TurnerMay 2, 2016
  26. 09/14 fetch-pack: Add negotiate_capabilitiesStefan Beller, Apr 29, 2016
  27. 10/14 do_fetch_pack: select capabilities for transport version 1 onlyStefan Beller, Apr 29, 2016
  28. 11/14 builtin/fetch-pack: add argument for transport versionStefan Beller, Apr 29, 2016
  29. 12/14 Add test for fetch-packStefan Beller, Apr 29, 2016
  30. David TurnerMay 2, 2016
  31. 13/14 WIP add test for git pullStefan Beller, Apr 29, 2016
  32. 14/14 WIP test git fetchStefan Beller, Apr 29, 2016
  33. David TurnerMay 2, 2016
  34. Stefan BellerMay 2, 2016
  35. David TurnerMay 24, 2016
  36. Duy NguyenMay 24, 2016
  37. David TurnerMay 25, 2016
  38. Junio C HamanoMay 25, 2016
  39. David TurnerMay 25, 2016
  40. Jeff KingMay 25, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.