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

Re: [PATCH 2/3] http-push: use hex_to_bytes()

From
René Scharfe <l.s.r@web.de>
Date
Nov 4, 2017, 09:05 UTC
Message-ID
<abf7c219-880b-790f-78ef-18c7cdd54ad6@web.de>
In-Reply-To
<20171101221541.6dwezd4t4hweuis2@sigill.intra.peff.net>
Am 01.11.2017 um 23:15 schrieb Jeff King:
Show 12 quoted lines
> On Wed, Nov 01, 2017 at 10:59:49PM +0100, René Scharfe wrote:
> 
>>> The hex_to_bytes() function requires that the caller make sure they have
>>> the right number of bytes. But for many callers, I think they'd want to
>>> say "parse this oid, which might be truncated; I can't tell what the
>>> length is supposed to be".
>>
>> I'm confused by the word "many".  After this series there are three
>> callers of hex_to_bytes() and I don't expect that number to grow.
> 
> I meant only that most callers that parse oids, both in-file and not,
> would want to stop knowing about the length ahead of time.
That's a good idea.
> I'm not sure we know 100%
> yet what "new"-style hashes will look like, nor how their loose-object
> filenames would look.

So it's too early to implement a solution, but here's how a patch for reducing dependency on hash lengths could look like today:

---
 cache.h     |  2 ++
 hex.c       | 13 +++++++++++++
 http-push.c |  7 +++----
 notes.c     |  6 +-----
 sha1_file.c |  4 +---
 5 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/cache.h b/cache.h
index f06bfbaf32..acd3804c21 100644
--- a/cache.h
+++ b/cache.h
@@ -1317,6 +1317,8 @@ extern int set_disambiguate_hint_config(const char *var, const char *value);
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern int get_oid_hex(const char *hex, struct object_id *sha1);
 
+extern int get_oid_hex_tail(const char *hex, struct object_id *oid, size_t offset);
+
 /*
  * Read `len` pairs of hexadecimal digits from `hex` and write the
  * values to `binary` as `len` bytes. Return 0 on success, or -1 if
diff --git a/hex.c b/hex.c
index 8df2d63728..3e6abe4d5e 100644
--- a/hex.c
+++ b/hex.c
@@ -47,6 +47,19 @@ int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)
 	return 0;
 }
 
+int get_oid_hex_tail(const char *hex, struct object_id *oid, size_t offset)
+{
+	for (; offset < GIT_SHA1_RAWSZ; offset++, hex += 2) {
+		int val = hex2chr(hex);
+		if (val < 0)
+			return -1;
+		oid->hash[offset] = val;
+	}
+	if (*hex)
+		return -1;
+	return 0;
+}
+
 int get_sha1_hex(const char *hex, unsigned char *sha1)
 {
 	int i;
diff --git a/http-push.c b/http-push.c
index 14435ab65d..a5512616b9 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1007,18 +1007,17 @@ static void remote_ls(const char *path, int flags,
 		      void (*userFunc)(struct remote_ls_ctx *ls),
 		      void *userData);
 
-/* extract hex from sharded "xx/x{38}" filename */
+/* extract hex from sharded "xx/x{N}" filename */
 static int get_oid_hex_from_objpath(const char *path, struct object_id *oid)
 {
-	if (strlen(path) != GIT_SHA1_HEXSZ + 1)
+	if (strnlen(path, 3) < 3)
 		return -1;
-
 	if (hex_to_bytes(oid->hash, path, 1))
 		return -1;
 	path += 2;
 	path++; /* skip '/' */
 
-	return hex_to_bytes(oid->hash + 1, path, GIT_SHA1_RAWSZ - 1);
+	return get_oid_hex_tail(path, oid, 1);
 }
 
 static void process_ls_object(struct remote_ls_ctx *ls)
diff --git a/notes.c b/notes.c
index 04f8c8613c..ff6ce57022 100644
--- a/notes.c
+++ b/notes.c
@@ -410,17 +410,13 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 		struct leaf_node *l;
 		size_t path_len = strlen(entry.path);
 
-		if (path_len == 2 * (GIT_SHA1_RAWSZ - prefix_len)) {
+		if (!get_oid_hex_tail(entry.path, &object_oid, prefix_len)) {
 			/* This is potentially the remainder of the SHA-1 */
 
 			if (!S_ISREG(entry.mode))
 				/* notes must be blobs */
 				goto handle_non_note;
 
-			if (hex_to_bytes(object_oid.hash + prefix_len, entry.path,
-					 GIT_SHA1_RAWSZ - prefix_len))
-				goto handle_non_note; /* entry.path is not a SHA1 */
-
 			type = PTR_TYPE_NOTE;
 		} else if (path_len == 2) {
 			/* This is potentially an internal node */
diff --git a/sha1_file.c b/sha1_file.c
index a3c32d91d1..0486696b0b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1911,9 +1911,7 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
 		strbuf_setlen(path, baselen);
 		strbuf_addf(path, "/%s", de->d_name);
 
-		if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 &&
-		    !hex_to_bytes(oid.hash + 1, de->d_name,
-				  GIT_SHA1_RAWSZ - 1)) {
+		if (!get_oid_hex_tail(de->d_name, &oid, 1)) {
 			if (obj_cb) {
 				r = obj_cb(&oid, path->buf, data);
 				if (r)
-- 
2.15.0
Previous: Jeff KingNext: René Scharfe
Message 6 of 11 in “notes: move hex_to_bytes() to hex.c and export it”
  1. 1/3 notes: move hex_to_bytes() to hex.c and export itRené Scharfe, Oct 31, 2017
  2. 2/3 http-push: use hex_to_bytes()René Scharfe, Oct 31, 2017
  3. Jeff KingNov 1, 2017
  4. René ScharfeNov 1, 2017
  5. Jeff KingNov 1, 2017
  6. René ScharfeNov 4, 2017
  7. 3/3 sha1_file: use hex_to_bytes()René Scharfe, Oct 31, 2017
  8. Jeff KingNov 1, 2017
  9. Kevin DaudtNov 5, 2017
  10. René ScharfeNov 5, 2017
  11. Kevin DaudtNov 5, 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.