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

[PATCH v2 7/9] upload-pack: refactor reading of pack-objects out

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Jun 10, 2020, 20:57 UTC
Message-ID
<927ca4133cc0941bd3c16367347e9833feced0cd.1591821067.git.jonathantanmy@google.com>
In-Reply-To
<cover.1591821067.git.jonathantanmy@google.com>

Subsequent patches will change how the output of pack-objects is processed, so extract that processing into its own function.

Currently, at most 1 character can be buffered (in the "buffered" local variable). One of those patches will require a larger buffer, so replace that "buffered" local variable with a buffer array.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 upload-pack.c | 84 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 49 insertions(+), 35 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index bc7e3ca19d..da1f749620 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -173,13 +173,52 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
 	return 0;
 }
 
+struct output_state {
+	char buffer[8193];
+	int used;
+};
+
+static int relay_pack_data(int pack_objects_out, struct output_state *os,
+			   int use_sideband)
+{
+	/*
+	 * We keep the last byte to ourselves
+	 * in case we detect broken rev-list, so that we
+	 * can leave the stream corrupted.  This is
+	 * unfortunate -- unpack-objects would happily
+	 * accept a valid packdata with trailing garbage,
+	 * so appending garbage after we pass all the
+	 * pack data is not good enough to signal
+	 * breakage to downstream.
+	 */
+	ssize_t readsz;
+
+	readsz = xread(pack_objects_out, os->buffer + os->used,
+		       sizeof(os->buffer) - os->used);
+	if (readsz < 0) {
+		return readsz;
+	}
+	os->used += readsz;
+
+	if (os->used > 1) {
+		send_client_data(1, os->buffer, os->used - 1, use_sideband);
+		os->buffer[0] = os->buffer[os->used - 1];
+		os->used = 1;
+	} else {
+		send_client_data(1, os->buffer, os->used, use_sideband);
+		os->used = 0;
+	}
+
+	return readsz;
+}
+
 static void create_pack_file(struct upload_pack_data *pack_data)
 {
 	struct child_process pack_objects = CHILD_PROCESS_INIT;
-	char data[8193], progress[128];
+	struct output_state output_state = { { 0 } };
+	char progress[128];
 	char abort_msg[] = "aborting due to possible repository "
 		"corruption on the remote side.";
-	int buffered = -1;
 	ssize_t sz;
 	int i;
 	FILE *pipe_fd;
@@ -312,40 +351,16 @@ static void create_pack_file(struct upload_pack_data *pack_data)
 			continue;
 		}
 		if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) {
-			/* Data ready; we keep the last byte to ourselves
-			 * in case we detect broken rev-list, so that we
-			 * can leave the stream corrupted.  This is
-			 * unfortunate -- unpack-objects would happily
-			 * accept a valid packdata with trailing garbage,
-			 * so appending garbage after we pass all the
-			 * pack data is not good enough to signal
-			 * breakage to downstream.
-			 */
-			char *cp = data;
-			ssize_t outsz = 0;
-			if (0 <= buffered) {
-				*cp++ = buffered;
-				outsz++;
-			}
-			sz = xread(pack_objects.out, cp,
-				  sizeof(data) - outsz);
-			if (0 < sz)
-				;
-			else if (sz == 0) {
+			int result = relay_pack_data(pack_objects.out,
+						     &output_state,
+						     pack_data->use_sideband);
+
+			if (result == 0) {
 				close(pack_objects.out);
 				pack_objects.out = -1;
-			}
-			else
+			} else if (result < 0) {
 				goto fail;
-			sz += outsz;
-			if (1 < sz) {
-				buffered = data[sz-1] & 0xFF;
-				sz--;
 			}
-			else
-				buffered = -1;
-			send_client_data(1, data, sz,
-					 pack_data->use_sideband);
 		}
 
 		/*
@@ -370,9 +385,8 @@ static void create_pack_file(struct upload_pack_data *pack_data)
 	}
 
 	/* flush the data */
-	if (0 <= buffered) {
-		data[0] = buffered;
-		send_client_data(1, data, 1,
+	if (output_state.used > 0) {
+		send_client_data(1, output_state.buffer, output_state.used,
 				 pack_data->use_sideband);
 		fprintf(stderr, "flushed.\n");
 	}
-- 
2.27.0.278.ge193c7cf3a9-goog
Previous: Ævar Arnfjörð BjarmasonNext: Jonathan Tan
Message 40 of 44 in “CDN offloading update”
  1. 0/8 CDN offloading updateJonathan Tan, May 29, 2020
  2. 1/8 http: use --stdin when getting dumb HTTP packJonathan Tan, May 29, 2020
  3. Junio C HamanoMay 29, 2020
  4. Jonathan TanJun 1, 2020
  5. 2/8 http: improve documentation of http_pack_requestJonathan Tan, May 29, 2020
  6. 4/8 Documentation: order protocol v2 sectionsJonathan Tan, May 29, 2020
  7. Junio C HamanoMay 29, 2020
  8. Jonathan TanJun 1, 2020
  9. 3/8 http-fetch: support fetching packfiles by URLJonathan Tan, May 29, 2020
  10. Junio C HamanoMay 29, 2020
  11. Jonathan TanJun 1, 2020
  12. 5/8 Documentation: add Packfile URIs design docJonathan Tan, May 29, 2020
  13. Junio C HamanoMay 30, 2020
  14. Junio C HamanoMay 30, 2020
  15. Jonathan TanJun 1, 2020
  16. Jonathan TanJun 1, 2020
  17. Jonathan TanJun 10, 2020
  18. Junio C HamanoJun 10, 2020
  19. Jonathan TanJun 10, 2020
  20. 6/8 upload-pack: refactor reading of pack-objects outJonathan Tan, May 29, 2020
  21. 8/8 upload-pack: send part of packfile response as uriJonathan Tan, May 29, 2020
  22. Junio C HamanoMay 31, 2020
  23. Junio C HamanoMay 31, 2020
  24. Jonathan TanJun 1, 2020
  25. Jonathan NiederJun 1, 2020
  26. 7/8 fetch-pack: support more than one pack lockfileJonathan Tan, May 29, 2020
  27. 0/9 CDN offloading updateJonathan Tan, Jun 10, 2020
  28. 1/9 http: use --stdin when indexing dumb HTTP packJonathan Tan, Jun 10, 2020
  29. Junio C HamanoJun 11, 2020
  30. 2/9 http: refactor finish_http_pack_request()Jonathan Tan, Jun 10, 2020
  31. 3/9 http-fetch: refactor into functionJonathan Tan, Jun 10, 2020
  32. 5/9 Documentation: order protocol v2 sectionsJonathan Tan, Jun 10, 2020
  33. 4/9 http-fetch: support fetching packfiles by URLJonathan Tan, Jun 10, 2020
  34. Junio C HamanoJun 11, 2020
  35. 6/9 Documentation: add Packfile URIs design docJonathan Tan, Jun 10, 2020
  36. Junio C HamanoJun 11, 2020
  37. Ævar Arnfjörð BjarmasonNov 25, 2020
  38. Jonathan TanNov 25, 2020
  39. Ævar Arnfjörð BjarmasonDec 1, 2020
  40. 7/9 upload-pack: refactor reading of pack-objects outJonathan Tan, Jun 10, 2020
  41. 8/9 fetch-pack: support more than one pack lockfileJonathan Tan, Jun 10, 2020
  42. Junio C HamanoJun 11, 2020
  43. 9/9 upload-pack: send part of packfile response as uriJonathan Tan, Jun 10, 2020
  44. Junio C HamanoJun 10, 2020

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.