"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
do not hardcode digest length when parsing packed trees
To:
gameoftrees@openbsd.org
Date:
Fri, 12 Jul 2024 15:33:56 +0200

Download raw body.

Thread
the subject says it all.  there are a few upcoming diff where I'm
replacing SHA1_DIGEST{,_STRING}_LENGTH with the right length derived
with got_hash_digest{,_string}_length() but this is a bit more delicate.
I know that this is a performance-sensitive area and that's why i'm
passing the idlen instead of having the function computing it.  to be
fair though i haven't done any benchmark.

ok?

diff -s /home/op/w/got
commit - 4239df3cca84ef6d17b350804eab03fa39f41d20
path + /home/op/w/got (staged changes)
blob - 746e561381b85c24e2b02e2298cff0dbbf3b9912
blob + f871e8cb69954a7bce76347100e63673a2c8fd95
--- lib/got_lib_object_parse.h
+++ lib/got_lib_object_parse.h
@@ -33,7 +33,7 @@ struct got_parsed_tree_entry {
 	uint8_t *id; /* Points to ID in parsed tree buffer. */
 };
 const struct got_error *got_object_parse_tree_entry(
-    struct got_parsed_tree_entry *, size_t *, char *, size_t);
+    struct got_parsed_tree_entry *, size_t *, char *, size_t, size_t);
 const struct got_error *got_object_parse_tree(struct got_parsed_tree_entry **,
     size_t *, size_t *, uint8_t *, size_t);
 const struct got_error *got_object_read_tree(struct got_parsed_tree_entry **,
blob - 2309e009560dfaa866d6ee8a0a268e8dbc61a114
blob + f3cba911b4349a33264333ee8d8e133f09221710
--- lib/object_parse.c
+++ lib/object_parse.c
@@ -762,7 +762,7 @@ got_object_tree_close(struct got_tree_object *tree)
 
 const struct got_error *
 got_object_parse_tree_entry(struct got_parsed_tree_entry *pte, size_t *elen,
-    char *buf, size_t maxlen)
+    char *buf, size_t maxlen, size_t idlen)
 {
 	char *p, *space;
 
@@ -786,14 +786,14 @@ got_object_parse_tree_entry(struct got_parsed_tree_ent
 		p++;
 	}
 
-	if (*elen > maxlen || maxlen - *elen < SHA1_DIGEST_LENGTH)
+	if (*elen > maxlen || maxlen - *elen < idlen)
 		return got_error(GOT_ERR_BAD_OBJ_DATA);
 
 	pte->name = space + 1;
 	pte->namelen = strlen(pte->name);
 	buf += *elen;
 	pte->id = buf;
-	*elen += SHA1_DIGEST_LENGTH;
+	*elen += idlen;
 	return NULL;
 }
 
@@ -810,11 +810,13 @@ got_object_parse_tree(struct got_parsed_tree_entry **e
     size_t *nentries_alloc, uint8_t *buf, size_t len)
 {
 	const struct got_error *err = NULL;
-	size_t remain = len;
+	size_t idlen, remain = len;
 	const size_t nalloc = 16;
 	struct got_parsed_tree_entry *pte;
 	int i;
 
+	idlen = got_hash_digest_length(GOT_HASH_SHA1);
+
 	*nentries = 0;
 	if (remain == 0)
 		return NULL; /* tree is empty */
@@ -834,7 +836,8 @@ got_object_parse_tree(struct got_parsed_tree_entry **e
 		}
 
 		pte = &(*entries)[*nentries];
-		err = got_object_parse_tree_entry(pte, &elen, buf, remain);
+		err = got_object_parse_tree_entry(pte, &elen, buf, remain,
+			idlen);
 		if (err)
 			goto done;
 		buf += elen;
blob - b4ec204772c2f95248eaebb7f4c5d724d535c17b
blob + e55cdb411d26905c5b5b6fcb06e7fe199e96521f
--- libexec/got-read-pack/got-read-pack.c
+++ libexec/got-read-pack/got-read-pack.c
@@ -40,6 +40,7 @@
 
 #include "got_lib_delta.h"
 #include "got_lib_delta_cache.h"
+#include "got_lib_hash.h"
 #include "got_lib_object.h"
 #include "got_lib_object_qid.h"
 #include "got_lib_object_cache.h"
@@ -452,7 +453,7 @@ tree_path_changed(int *changed, uint8_t **buf1, size_t
 	const struct got_error *err = NULL;
 	struct got_parsed_tree_entry pte1, pte2;
 	const char *seg, *s;
-	size_t seglen;
+	size_t seglen, idlen;
 	size_t remain1 = *len1, remain2 = *len2, elen;
 	uint8_t *next_entry1 = *buf1;
 	uint8_t *next_entry2 = *buf2;
@@ -462,6 +463,8 @@ tree_path_changed(int *changed, uint8_t **buf1, size_t
 
 	*changed = 0;
 
+	idlen = got_hash_digest_length(GOT_HASH_SHA1);
+
 	/* We not do support comparing the root path. */
 	if (got_path_is_root_dir(path))
 		return got_error_path(path, GOT_ERR_BAD_PATH);
@@ -492,7 +495,7 @@ tree_path_changed(int *changed, uint8_t **buf1, size_t
 		 */
 		while (remain1 > 0) {
 			err = got_object_parse_tree_entry(&pte1, &elen,
-			    next_entry1, remain1);
+			    next_entry1, remain1, idlen);
 			if (err)
 				return err;
 			next_entry1 += elen;
@@ -516,7 +519,7 @@ tree_path_changed(int *changed, uint8_t **buf1, size_t
 
 		while (remain2 > 0) {
 			err = got_object_parse_tree_entry(&pte2, &elen,
-			    next_entry2, remain2);
+			    next_entry2, remain2, idlen);
 			if (err)
 				return err;
 			next_entry2 += elen;