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

[WIP 7/4] upload-pack: refactor reading of pack-objects out

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Jan 11, 2019, 22:18 UTC
Message-ID
<e87b45637a9773f0aaf1a04e97c0939e45a0cf80.1547244620.git.jonathantanmy@google.com>
In-Reply-To
<cover.1547244620.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 | 80 +++++++++++++++++++++++++++++----------------------
 1 file changed, 46 insertions(+), 34 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index 0c1feccaab..c87b752550 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -103,14 +103,51 @@ 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 read_pack_objects_stdout(int outfd, struct output_state *os)
+{
+	/* 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.
+	 */
+	ssize_t readsz;
+
+	readsz = xread(outfd, 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);
+		os->buffer[0] = os->buffer[os->used - 1];
+		os->used = 1;
+	} else {
+		send_client_data(1, os->buffer, os->used);
+		os->used = 0;
+	}
+
+	return readsz;
+}
+
 static void create_pack_file(const struct object_array *have_obj,
 			     const struct object_array *want_obj)
 {
 	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;
@@ -237,39 +274,15 @@ static void create_pack_file(const struct object_array *have_obj,
 			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 = read_pack_objects_stdout(pack_objects.out,
+							      &output_state);
+
+			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);
 		}
 
 		/*
@@ -294,9 +307,8 @@ static void create_pack_file(const struct object_array *have_obj,
 	}
 
 	/* 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);
 		fprintf(stderr, "flushed.\n");
 	}
 	if (use_sideband)
-- 
2.19.0.271.gfe8321ec05.dirty
Previous: Jonathan TanNext: Jonathan Tan
Message 16 of 41 in “Sideband the whole fetch v2 response”
  1. 0/4 Sideband the whole fetch v2 responseJonathan Tan, Jan 11, 2019
  2. 1/4 pkt-line: introduce struct packet_writerJonathan Tan, Jan 11, 2019
  3. Junio C HamanoJan 14, 2019
  4. Jonathan TanJan 15, 2019
  5. 2/4 sideband: reverse its dependency on pkt-lineJonathan Tan, Jan 11, 2019
  6. Junio C HamanoJan 14, 2019
  7. Junio C HamanoJan 14, 2019
  8. Jonathan TanJan 15, 2019
  9. Jonathan TanJan 15, 2019
  10. 3/4 {fetch,upload}-pack: sideband v2 fetch responseJonathan Tan, Jan 11, 2019
  11. Junio C HamanoJan 14, 2019
  12. Jonathan TanJan 15, 2019
  13. 4/4 tests: define GIT_TEST_SIDEBAND_ALLJonathan Tan, Jan 11, 2019
  14. 5/4 Documentation: order protocol v2 sectionsJonathan Tan, Jan 11, 2019
  15. 6/4 Documentation: add Packfile URIs design docJonathan Tan, Jan 11, 2019
  16. 7/4 upload-pack: refactor reading of pack-objects outJonathan Tan, Jan 11, 2019
  17. 8/4 upload-pack: send part of packfile response as uriJonathan Tan, Jan 11, 2019
  18. 0/4 Sideband the whole fetch v2 responseJonathan Tan, Jan 15, 2019
  19. 1/4 pkt-line: introduce struct packet_writerJonathan Tan, Jan 15, 2019
  20. 2/4 sideband: reverse its dependency on pkt-lineJonathan Tan, Jan 15, 2019
  21. SZEDER GáborJan 16, 2019
  22. Jonathan TanJan 16, 2019
  23. SZEDER GáborJan 16, 2019
  24. 3/4 {fetch,upload}-pack: sideband v2 fetch responseJonathan Tan, Jan 15, 2019
  25. 4/4 tests: define GIT_TEST_SIDEBAND_ALLJonathan Tan, Jan 15, 2019
  26. Junio C HamanoJan 15, 2019
  27. Junio C HamanoJan 15, 2019
  28. Jonathan TanJan 15, 2019
  29. Junio C HamanoJan 15, 2019
  30. Jonathan TanJan 15, 2019
  31. Junio C HamanoJan 15, 2019
  32. Jonathan TanJan 16, 2019
  33. Junio C HamanoJan 16, 2019
  34. 0/4 Sideband the whole fetch v2 responseJonathan Tan, Jan 16, 2019
  35. 1/4 pkt-line: introduce struct packet_writerJonathan Tan, Jan 16, 2019
  36. 2/4 sideband: reverse its dependency on pkt-lineJonathan Tan, Jan 16, 2019
  37. 3/4 {fetch,upload}-pack: sideband v2 fetch responseJonathan Tan, Jan 16, 2019
  38. 4/4 tests: define GIT_TEST_SIDEBAND_ALLJonathan Tan, Jan 16, 2019
  39. Jonathan NiederJan 29, 2019
  40. Jeff KingFeb 13, 2019
  41. Junio C HamanoJan 17, 2019

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.