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

[PATCH 12/18] receive-pack: print --pack-header directly into argv array

From
Jeff King <peff@peff.net>
Date
Mar 28, 2017, 19:46 UTC
Message-ID
<20170328194647.ac4rj3aw7jbteqjh@sigill.intra.peff.net>
In-Reply-To
<20170328194255.vf7nfzzmmzxsbn36@sigill.intra.peff.net>

After receive-pack reads the pack header from the client, it feeds the already-read part to index-pack and unpack-objects via their --pack-header command-line options. To do so, we format it into a fixed buffer, then duplicate it into the child's argv_array.

Our buffer is long enough to handle any possible input, so this isn't wrong. But it's more complicated than it needs to be; we can just argv_array_pushf() the final value and avoid the intermediate copy. This drops the magic number and is more efficient, too.

Note that we need to push to the argv_array in order, which means we can't do the push until we are in the "unpack-objects versus index-pack" conditional. Rather than duplicate the slightly complicated format specifier, I pushed it into a helper function.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/receive-pack.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index fb2a090a0..2ca93adef 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1634,12 +1634,17 @@ static const char *parse_pack_header(struct pack_header *hdr)
 
 static const char *pack_lockfile;
 
+static void push_header_arg(struct argv_array *args, struct pack_header *hdr)
+{
+	argv_array_pushf(args, "--pack_header=%"PRIu32",%"PRIu32,
+			ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
+}
+
 static const char *unpack(int err_fd, struct shallow_info *si)
 {
 	struct pack_header hdr;
 	const char *hdr_err;
 	int status;
-	char hdr_arg[38];
 	struct child_process child = CHILD_PROCESS_INIT;
 	int fsck_objects = (receive_fsck_objects >= 0
 			    ? receive_fsck_objects
@@ -1653,9 +1658,6 @@ static const char *unpack(int err_fd, struct shallow_info *si)
 			close(err_fd);
 		return hdr_err;
 	}
-	snprintf(hdr_arg, sizeof(hdr_arg),
-			"--pack_header=%"PRIu32",%"PRIu32,
-			ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries));
 
 	if (si->nr_ours || si->nr_theirs) {
 		alt_shallow_file = setup_temporary_shallow(si->shallow);
@@ -1679,7 +1681,8 @@ static const char *unpack(int err_fd, struct shallow_info *si)
 	tmp_objdir_add_as_alternate(tmp_objdir);
 
 	if (ntohl(hdr.hdr_entries) < unpack_limit) {
-		argv_array_pushl(&child.args, "unpack-objects", hdr_arg, NULL);
+		argv_array_push(&child.args, "unpack-objects");
+		push_header_arg(&child.args, &hdr);
 		if (quiet)
 			argv_array_push(&child.args, "-q");
 		if (fsck_objects)
@@ -1697,8 +1700,8 @@ static const char *unpack(int err_fd, struct shallow_info *si)
 	} else {
 		char hostname[256];
 
-		argv_array_pushl(&child.args, "index-pack",
-				 "--stdin", hdr_arg, NULL);
+		argv_array_pushl(&child.args, "index-pack", "--stdin", NULL);
+		push_header_arg(&child.args, &hdr);
 
 		if (gethostname(hostname, sizeof(hostname)))
 			xsnprintf(hostname, sizeof(hostname), "localhost");
-- 
2.12.2.845.g55fcf8b10
Previous: Jeff KingNext: Jeff King
Message 12 of 30 in “snprintf cleanups”
  1. 0/18 snprintf cleanupsJeff King, Mar 28, 2017
  2. 01/18 do not check odb_mkstemp return value for errorsJeff King, Mar 28, 2017
  3. 02/18 odb_mkstemp: write filename into strbufJeff King, Mar 28, 2017
  4. 03/18 odb_mkstemp: use git_path_bufJeff King, Mar 28, 2017
  5. 05/18 tag: use strbuf to format tag headerJeff King, Mar 28, 2017
  6. 09/18 create_branch: move msg setup closer to point of useJeff King, Mar 28, 2017
  7. 08/18 avoid using mksnpath for refsJeff King, Mar 28, 2017
  8. 06/18 fetch: use heap buffer to format reflogJeff King, Mar 28, 2017
  9. 04/18 diff: avoid fixed-size buffer for patch-idsJeff King, Mar 28, 2017
  10. Jeff KingMar 28, 2017
  11. 13/18 replace unchecked snprintf calls with heap buffersJeff King, Mar 28, 2017
  12. 12/18 receive-pack: print --pack-header directly into argv arrayJeff King, Mar 28, 2017
  13. 07/18 avoid using fixed PATH_MAX buffers for refsJeff King, Mar 28, 2017
  14. Junio C HamanoApr 17, 2017
  15. Jeff KingApr 18, 2017
  16. Junio C HamanoApr 18, 2017
  17. 11/18 name-rev: replace static buffer with strbufJeff King, Mar 28, 2017
  18. 15/18 convert unchecked snprintf into xsnprintfJeff King, Mar 28, 2017
  19. 10/18 create_branch: use xstrfmt for reflog messageJeff King, Mar 28, 2017
  20. 14/18 combine-diff: replace malloc/snprintf with xstrfmtJeff King, Mar 28, 2017
  21. 17/18 gc: replace local buffer with git_pathJeff King, Mar 28, 2017
  22. 16/18 transport-helper: replace checked snprintf with xsnprintfJeff King, Mar 28, 2017
  23. 18/18 daemon: use an argv_array to exec childrenJeff King, Mar 28, 2017
  24. Junio C HamanoMar 28, 2017
  25. Jeff KingMar 29, 2017
  26. Junio C HamanoMar 29, 2017
  27. Jeff KingMar 30, 2017
  28. Junio C HamanoMar 30, 2017
  29. Jeff KingMar 30, 2017
  30. Makefile: detect errors in running spatchJeff King, Mar 29, 2017

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.