From: Stefan Sperling Subject: Re: struct vs buffer for got_imsg_{commit_,}object To: Omar Polo Cc: gameoftrees@openbsd.org Date: Wed, 1 Feb 2023 16:41:12 +0100 On Wed, Feb 01, 2023 at 04:12:53PM +0100, Omar Polo wrote: > separate diffs for easier reading (hopefully.) the first one is > trivial, the second one seems so but it's slightly tricky because in > some structs the "tree_id" field is the struct in others the pointer > to the struct: got_imsg_commit_object embeds the struct while > got_commit_object dosen't. > > ok? ok > commit 42c43229569244a64062c0b12fd11eef28120da8 > from: Omar Polo > date: Wed Feb 1 15:02:02 2023 UTC > > got_imsg_object: use struct instead of buffer for id > > diff 38cd26a413a43dc0a7d5f80041fd705fe0d55773 42c43229569244a64062c0b12fd11eef28120da8 > commit - 38cd26a413a43dc0a7d5f80041fd705fe0d55773 > commit + 42c43229569244a64062c0b12fd11eef28120da8 > blob - 151e0d73fa443289c8609863654c6bef6d128723 > blob + 2817ac884ce25ec43a9812d10cca3e9dc017a4cf > --- lib/got_lib_privsep.h > +++ lib/got_lib_privsep.h > @@ -220,7 +220,7 @@ struct got_imsg_object { > * Structure for GOT_IMSG_TREE_REQUEST and GOT_IMSG_OBJECT data. > */ > struct got_imsg_object { > - uint8_t id[SHA1_DIGEST_LENGTH]; > + struct got_object_id id; > > /* These fields are the same as in struct got_object. */ > int type; > blob - 970978560467422ed3a634b6c3b86e48e6b49fd0 > blob + 1f118154617c4dae7a5a12d5531b577d6d906da7 > --- lib/privsep.c > +++ lib/privsep.c > @@ -398,7 +398,7 @@ got_privsep_send_tree_req(struct imsgbuf *ibuf, int fd > if (wbuf == NULL) > return got_error_from_errno("imsg_create TREE_REQUEST"); > > - if (imsg_add(wbuf, id->sha1, SHA1_DIGEST_LENGTH) == -1) > + if (imsg_add(wbuf, id, sizeof(*id)) == -1) > return got_error_from_errno("imsg_add TREE_REQUEST"); > > if (pack_idx != -1) { /* tree is packed */ > @@ -510,7 +510,7 @@ got_privsep_send_obj(struct imsgbuf *ibuf, struct got_ > > memset(&iobj, 0, sizeof(iobj)); > > - memcpy(iobj.id, obj->id.sha1, sizeof(iobj.id)); > + memcpy(&iobj.id, &obj->id, sizeof(iobj.id)); > iobj.type = obj->type; > iobj.flags = obj->flags; > iobj.hdrlen = obj->hdrlen; > @@ -1118,7 +1118,7 @@ got_privsep_get_imsg_obj(struct got_object **obj, stru > if (*obj == NULL) > return got_error_from_errno("calloc"); > > - memcpy((*obj)->id.sha1, iobj->id, SHA1_DIGEST_LENGTH); > + memcpy(&(*obj)->id, &iobj->id, sizeof(iobj->id)); > (*obj)->type = iobj->type; > (*obj)->flags = iobj->flags; > (*obj)->hdrlen = iobj->hdrlen; > > ----------------------------------------------- > commit 0c1022cbc2057b7964c07168452299432782b57b (main) > from: Omar Polo > date: Wed Feb 1 15:02:02 2023 UTC > > got_imsg_commit_object: use struct instead of buffer for id > > diff 42c43229569244a64062c0b12fd11eef28120da8 0c1022cbc2057b7964c07168452299432782b57b > commit - 42c43229569244a64062c0b12fd11eef28120da8 > commit + 0c1022cbc2057b7964c07168452299432782b57b > blob - 2817ac884ce25ec43a9812d10cca3e9dc017a4cf > blob + 1b4b7bc8fc2baddfd1f3353725f35bd6ac9af0cf > --- lib/got_lib_privsep.h > +++ lib/got_lib_privsep.h > @@ -233,7 +233,7 @@ struct got_imsg_commit_object { > > /* Structure for GOT_IMSG_COMMIT data. */ > struct got_imsg_commit_object { > - uint8_t tree_id[SHA1_DIGEST_LENGTH]; > + struct got_object_id tree_id; > size_t author_len; > time_t author_time; > time_t author_gmtoff; > @@ -247,7 +247,7 @@ struct got_imsg_commit_object { > * Followed by author_len + committer_len data bytes > */ > > - /* Followed by 'nparents' SHA1_DIGEST_LENGTH length strings */ > + /* Followed by 'nparents' struct got_object_id */ > > /* > * Followed by 'logmsg_len' bytes of commit log message data in > blob - 1f118154617c4dae7a5a12d5531b577d6d906da7 > blob + d40d65ea1e93b234a672d9edd3fef1c550898939 > --- lib/privsep.c > +++ lib/privsep.c > @@ -1203,15 +1203,14 @@ got_privsep_send_commit(struct imsgbuf *ibuf, struct g > size_t logmsg_len = strlen(commit->logmsg); > > total = sizeof(*icommit) + author_len + committer_len + > - commit->nparents * SHA1_DIGEST_LENGTH; > + commit->nparents * sizeof(struct got_object_id); > > buf = malloc(total); > if (buf == NULL) > return got_error_from_errno("malloc"); > > icommit = (struct got_imsg_commit_object *)buf; > - memcpy(icommit->tree_id, commit->tree_id->sha1, > - sizeof(icommit->tree_id)); > + memcpy(&icommit->tree_id, commit->tree_id, sizeof(icommit->tree_id)); > icommit->author_len = author_len; > icommit->author_time = commit->author_time; > icommit->author_gmtoff = commit->author_gmtoff; > @@ -1227,8 +1226,8 @@ got_privsep_send_commit(struct imsgbuf *ibuf, struct g > memcpy(buf + len, commit->committer, committer_len); > len += committer_len; > STAILQ_FOREACH(qid, &commit->parent_ids, entry) { > - memcpy(buf + len, &qid->id, SHA1_DIGEST_LENGTH); > - len += SHA1_DIGEST_LENGTH; > + memcpy(buf + len, &qid->id, sizeof(qid->id)); > + len += sizeof(qid->id); > } > > if (imsg_compose(ibuf, GOT_IMSG_COMMIT, 0, 0, -1, buf, len) == -1) { > @@ -1263,7 +1262,7 @@ get_commit_from_imsg(struct got_commit_object **commit > icommit = imsg->data; > if (datalen != sizeof(*icommit) + icommit->author_len + > icommit->committer_len + > - icommit->nparents * SHA1_DIGEST_LENGTH) > + icommit->nparents * sizeof(struct got_object_id)) > return got_error(GOT_ERR_PRIVSEP_LEN); > > if (icommit->nparents < 0) > @@ -1276,8 +1275,8 @@ get_commit_from_imsg(struct got_commit_object **commit > return got_error_from_errno( > "got_object_commit_alloc_partial"); > > - memcpy((*commit)->tree_id->sha1, icommit->tree_id, > - SHA1_DIGEST_LENGTH); > + memcpy((*commit)->tree_id, &icommit->tree_id, > + sizeof(icommit->tree_id)); > (*commit)->author_time = icommit->author_time; > (*commit)->author_gmtoff = icommit->author_gmtoff; > (*commit)->committer_time = icommit->committer_time; > @@ -1342,7 +1341,7 @@ get_commit_from_imsg(struct got_commit_object **commit > if (err) > break; > memcpy(&qid->id, imsg->data + len + > - i * SHA1_DIGEST_LENGTH, sizeof(qid->id)); > + i * sizeof(qid->id), sizeof(qid->id)); > STAILQ_INSERT_TAIL(&(*commit)->parent_ids, qid, entry); > (*commit)->nparents++; > } > >