From: Omar Polo Subject: struct vs buffers for got_imsg_tree_entry To: gameoftrees@openbsd.org Date: Wed, 01 Feb 2023 17:55:15 +0100 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)++;