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

[PATCH 17/16] http-push: refactor parsing of remote object names

From
Jeff King <peff@peff.net>
Date
Jun 19, 2014, 21:58 UTC
Message-ID
<20140619215810.GA29685@sigill.intra.peff.net>
In-Reply-To
<20140618194117.GA22269@sigill.intra.peff.net>

We get loose object names like "objects/??/..." from the remote side, and need to convert them to their hex representation.

The code to do so is rather hard to follow, as it uses some calculated lengths whose origins are hard to understand and verify (e.g., the path must be exactly 49 characters long. why? Why doesn't the strcpy overflow obj_hex, which is the same length as path?).

We can simplify this a bit by using skip_prefix, using standard 40- and 20-character buffers for hex and binary sha1s, and adding some comments.

We also drop a totally bogus comment that claims strlcpy cannot be used because "path" is not NUL-terminated. Right between a call to strlen(path) and strcpy(path).

Signed-off-by: Jeff King <peff@peff.net>
---
I found this one while doing the xstrfmt series, but it actually doesn't
need xstrfmt, and does need skip_prefix, so it probably goes better on
the skip-prefix topic.

It's still a little more magical than I would like, but I think this is the best we can do while still building on get_sha1_hex. Parsing it left-to-right would be better, but we would essentially end up reimplementing get_sha1_hex.

 http-push.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/http-push.c b/http-push.c
index 26dfa67..c5c95e8 100644
--- a/http-push.c
+++ b/http-push.c
@@ -719,14 +719,10 @@ static int fetch_indices(void)
 	return ret;
 }
 
-static void one_remote_object(const char *hex)
+static void one_remote_object(const unsigned char *sha1)
 {
-	unsigned char sha1[20];
 	struct object *obj;
 
-	if (get_sha1_hex(hex, sha1) != 0)
-		return;
-
 	obj = lookup_object(sha1);
 	if (!obj)
 		obj = parse_object(sha1);
@@ -1020,26 +1016,38 @@ static void remote_ls(const char *path, int flags,
 		      void (*userFunc)(struct remote_ls_ctx *ls),
 		      void *userData);
 
+/* extract hex from sharded "xx/x{40}" filename */
+static int get_sha1_hex_from_objpath(const char *path, unsigned char *sha1)
+{
+	char hex[40];
+
+	if (strlen(path) != 41)
+		return -1;
+
+	memcpy(hex, path, 2);
+	path += 2;
+	path++; /* skip '/' */
+	memcpy(hex, path, 38);
+
+	return get_sha1_hex(hex, sha1);
+}
+
 static void process_ls_object(struct remote_ls_ctx *ls)
 {
 	unsigned int *parent = (unsigned int *)ls->userData;
-	char *path = ls->dentry_name;
-	char *obj_hex;
+	const char *path = ls->dentry_name;
+	unsigned char sha1[20];
 
 	if (!strcmp(ls->path, ls->dentry_name) && (ls->flags & IS_DIR)) {
 		remote_dir_exists[*parent] = 1;
 		return;
 	}
 
-	if (strlen(path) != 49)
+	if (!skip_prefix(path, "objects/", &path) ||
+	    get_sha1_hex_from_objpath(path, sha1))
 		return;
-	path += 8;
-	obj_hex = xmalloc(strlen(path));
-	/* NB: path is not null-terminated, can not use strlcpy here */
-	memcpy(obj_hex, path, 2);
-	strcpy(obj_hex + 2, path + 3);
-	one_remote_object(obj_hex);
-	free(obj_hex);
+
+	one_remote_object(sha1);
 }
 
 static void process_ls_ref(struct remote_ls_ctx *ls)
-- 
2.0.0.566.gfe3e6b2
Previous: Tanay AbhraNext: Jeff King
Message 32 of 33 in “skip_prefix refactoring and cleanups”
  1. 0/16 skip_prefix refactoring and cleanupsJeff King, Jun 18, 2014
  2. 01/16 parse_diff_color_slot: drop ofs parameterJeff King, Jun 18, 2014
  3. 02/16 daemon: mark some strings as constJeff King, Jun 18, 2014
  4. 03/16 avoid using skip_prefix as a booleanJeff King, Jun 18, 2014
  5. 04/16 refactor skip_prefix to return a booleanJeff King, Jun 18, 2014
  6. Eric SunshineJun 20, 2014
  7. Jeff KingJun 20, 2014
  8. Eric SunshineJun 20, 2014
  9. Jeff KingJun 20, 2014
  10. Junio C HamanoJun 23, 2014
  11. Jeff KingJun 23, 2014
  12. builtin/clone.c: detect a clone starting at a tag correctlyJunio C Hamano, Jun 23, 2014
  13. 05/16 apply: use skip_prefix instead of raw additionJeff King, Jun 18, 2014
  14. 06/16 fast-import: fix read of uninitialized argv memoryJeff King, Jun 18, 2014
  15. 07/16 transport-helper: avoid reading past end-of-stringJeff King, Jun 18, 2014
  16. 08/16 use skip_prefix to avoid magic numbersJeff King, Jun 18, 2014
  17. Junio C HamanoJun 23, 2014
  18. Jeff KingJul 1, 2014
  19. 09/16 use skip_prefix to avoid repeating stringsJeff King, Jun 18, 2014
  20. 10/16 fast-import: use skip_prefix for parsing inputJeff King, Jun 18, 2014
  21. Eric SunshineJun 20, 2014
  22. Jeff KingJun 20, 2014
  23. Eric SunshineJun 20, 2014
  24. 11/16 daemon: use skip_prefix to avoid magic numbersJeff King, Jun 18, 2014
  25. 12/16 stat_opt: check extra strlen callJeff King, Jun 18, 2014
  26. 13/16 fast-import: refactor parsing of spacesJeff King, Jun 18, 2014
  27. 14/16 fetch-pack: refactor parsing in get_ackJeff King, Jun 18, 2014
  28. Eric SunshineJun 20, 2014
  29. 15/16 git: avoid magic number with skip_prefixJeff King, Jun 18, 2014
  30. 16/16 use skip_prefix to avoid repeated calculationsJeff King, Jun 18, 2014
  31. Tanay AbhraJun 19, 2014
  32. 17/16 http-push: refactor parsing of remote object namesJeff King, Jun 19, 2014
  33. Jeff KingJun 19, 2014

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.