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

From:
Omar Polo <op@omarpolo.com>
Subject:
struct vs buffers for got_imsg_tree_entry
To:
gameoftrees@openbsd.org
Date:
Wed, 01 Feb 2023 17:55:15 +0100

Download raw body.

Thread
Not as straightforward as the other diffs. The struct
got_parse_tree_entry keeps a pointer to the ID in the parsed tree
buffer so we need to reconstruct the got_object_id from that.  I've
done so in the function send_tree_entries_batch, although we could
also do that directly when parsing.  I went with the former to avoid
going thru the rabbit hole of changing got_parsed_tree_entry too.
We'll get to that later.

This is needed because I'd like to transmit only structs got_object_id
over imsg and don't want to run into issue should that struct be
changed (*wink*).  The downside is that we introduce a memcpy
(actually, double, imsg_add is another implcit memcpy) that might
impact runtime.

While here, i've also simplified a bit how we compute the size of the
entry.

diff 472dfe052943732fea89236959063c8ccdb769aa 8fbbe35390f3f6115907143af7099b9d6d8753be
commit - 472dfe052943732fea89236959063c8ccdb769aa
commit + 8fbbe35390f3f6115907143af7099b9d6d8753be
blob - 1b4b7bc8fc2baddfd1f3353725f35bd6ac9af0cf
blob + e8faa3304ae239f5025dc686c81c2731c1b935b6
--- lib/got_lib_privsep.h
+++ lib/got_lib_privsep.h
@@ -256,7 +256,7 @@ struct got_imsg_tree_entry {
 } __attribute__((__packed__));
 
 struct got_imsg_tree_entry {
-	char id[SHA1_DIGEST_LENGTH];
+	struct got_object_id id;
 	mode_t mode;
 	size_t namelen;
 	/* Followed by namelen bytes of entry's name, not NUL-terminated. */
blob - d40d65ea1e93b234a672d9edd3fef1c550898939
blob + b13d23747f95ae4601a3823676efd2fa125e0cb8
--- lib/privsep.c
+++ lib/privsep.c
@@ -1405,9 +1405,13 @@ send_tree_entries_batch(struct imsgbuf *ibuf,
 
 	for (i = idx0; i <= idxN; i++) {
 		struct got_parsed_tree_entry *pte = &entries[i];
+		struct got_object_id id;
 
+		memset(&id, 0, sizeof(id));
+		memcpy(&id.sha1, pte->id, SHA1_DIGEST_LENGTH);
+
 		/* Keep in sync with struct got_imsg_tree_object definition! */
-		if (imsg_add(wbuf, pte->id, SHA1_DIGEST_LENGTH) == -1)
+		if (imsg_add(wbuf, &id, sizeof(id)) == -1)
 			return got_error_from_errno("imsg_add TREE_ENTRY");
 		if (imsg_add(wbuf, &pte->mode, sizeof(pte->mode)) == -1)
 			return got_error_from_errno("imsg_add TREE_ENTRY");
@@ -1435,8 +1439,7 @@ send_tree_entries(struct imsgbuf *ibuf, struct got_par
 	i = 0;
 	for (j = 0; j < nentries; j++) {
 		struct got_parsed_tree_entry *pte = &entries[j];
-		size_t len = SHA1_DIGEST_LENGTH + sizeof(pte->mode) +
-		    sizeof(pte->namelen) + pte->namelen;
+		size_t len = sizeof(pte) + pte->namelen;
 
 		if (j > 0 &&
 		    entries_len + len > MAX_IMSGSIZE - IMSG_HEADER_SIZE) {
@@ -1533,7 +1536,7 @@ recv_tree_entries(void *data, size_t datalen, struct g
 		te_name = buf + sizeof(ite);
 		memcpy(te->name, te_name, ite.namelen);
 		te->name[ite.namelen] = '\0';
-		memcpy(te->id.sha1, ite.id, SHA1_DIGEST_LENGTH);
+		memcpy(&te->id, &ite.id, sizeof(te->id));
 		te->mode = ite.mode;
 		te->idx = *nentries;
 		(*nentries)++;