Download raw body.
got_privsep_recv_tree tweaks and more cosmetic stuff
On Thu, Jul 07, 2022 at 01:24:20PM +0200, Omar Polo wrote:
> while playing with the idea of splitting privsep.c into smaller pieces I
> noted that there are a few places where we call recv_imsg_error. Well,
> got_privsep_recv_imsg already turns those messages into errors, so it's
> not needed.
>
> While removing the vorious calls to recv_imsg_error i stumbled upon
> got_privsep_recv_tree too, which I think could be simplified a bit and
> made to work using got_privsep_recv_imsg. (note that this is not a
> purely cosmetic issue; imsg_get could return -1 and that is not
> handled.)
>
> Also, use got_privsep_flush_imsg instead of reinventing it in the patch
> code.
>
> Bundling these changes together because the diff is small. regress
> passes with GOT_TEST_PACK enabled and not.
Thanks, I like these simplifications a lot. Ok.
> diff refs/remotes/got/main refs/heads/main
> commit - 1df44de6f73549388bc170ad065645811ab002d0
> commit + 37f6390ec7dc516711cad5e321d83b7d278d6d0b
> blob - dbbc9b38387c388c49654f106cab82c723f3dd06
> blob + 703c22b84de200f2a0ec9fc0f7bdc70dc725ab20
> --- lib/patch.c
> +++ lib/patch.c
> @@ -100,12 +100,7 @@ send_patch(struct imsgbuf *ibuf, int fd)
> return err;
> }
>
> - if (imsg_flush(ibuf) == -1) {
> - err = got_error_from_errno("imsg_flush");
> - imsg_clear(ibuf);
> - }
> -
> - return err;
> + return got_privsep_flush_imsg(ibuf);
> }
>
> static void
> blob - ee575dbf080d9b5e9f2a1019a20d281907f06c91
> blob + 22b5fe916ef3380f1ead6796d6ddd366191190a2
> --- lib/privsep.c
> +++ lib/privsep.c
> @@ -710,9 +710,6 @@ got_privsep_recv_fetch_progress(int *done, struct got_
>
> datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
> switch (imsg.hdr.type) {
> - case GOT_IMSG_ERROR:
> - err = recv_imsg_error(&imsg, datalen);
> - break;
> case GOT_IMSG_FETCH_SYMREFS:
> if (datalen < sizeof(*isymrefs)) {
> err = got_error(GOT_ERR_PRIVSEP_LEN);
> @@ -934,9 +931,6 @@ got_privsep_recv_send_remote_refs(struct got_pathlist_
> return err;
> datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
> switch (imsg.hdr.type) {
> - case GOT_IMSG_ERROR:
> - err = recv_imsg_error(&imsg, datalen);
> - goto done;
> case GOT_IMSG_SEND_REMOTE_REF:
> if (datalen < sizeof(iremote_ref)) {
> err = got_error(GOT_ERR_PRIVSEP_MSG);
> @@ -1017,9 +1011,6 @@ got_privsep_recv_send_progress(int *done, off_t *bytes
>
> datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
> switch (imsg.hdr.type) {
> - case GOT_IMSG_ERROR:
> - err = recv_imsg_error(&imsg, datalen);
> - break;
> case GOT_IMSG_SEND_UPLOAD_PROGRESS:
> if (datalen < sizeof(*bytes_sent)) {
> err = got_error(GOT_ERR_PRIVSEP_MSG);
> @@ -1100,9 +1091,6 @@ got_privsep_recv_index_progress(int *done, int *nobj_t
>
> datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
> switch (imsg.hdr.type) {
> - case GOT_IMSG_ERROR:
> - err = recv_imsg_error(&imsg, datalen);
> - break;
> case GOT_IMSG_IDXPACK_PROGRESS:
> if (datalen < sizeof(*iprogress)) {
> err = got_error(GOT_ERR_PRIVSEP_LEN);
> @@ -1592,43 +1580,17 @@ got_privsep_recv_tree(struct got_tree_object **tree, s
>
> *tree = NULL;
>
> - err = read_imsg(ibuf);
> - if (err)
> - goto done;
> -
> - for (;;) {
> + while (*tree == NULL || nentries < (*tree)->nentries) {
> struct imsg imsg;
> - size_t n;
> size_t datalen;
>
> - n = imsg_get(ibuf, &imsg);
> - if (n == 0) {
> - if ((*tree)) {
> - if (nentries < (*tree)->nentries) {
> - err = read_imsg(ibuf);
> - if (err)
> - break;
> - continue;
> - } else
> - break;
> - } else {
> - err = got_error(GOT_ERR_PRIVSEP_MSG);
> - break;
> - }
> - }
> -
> - if (imsg.hdr.len < IMSG_HEADER_SIZE + min_datalen) {
> - imsg_free(&imsg);
> - err = got_error(GOT_ERR_PRIVSEP_LEN);
> + err = got_privsep_recv_imsg(&imsg, ibuf, min_datalen);
> + if (err)
> break;
> - }
>
> datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
>
> switch (imsg.hdr.type) {
> - case GOT_IMSG_ERROR:
> - err = recv_imsg_error(&imsg, datalen);
> - break;
> case GOT_IMSG_TREE:
> /* This message should only appear once. */
> if (*tree != NULL) {
> @@ -1678,7 +1640,7 @@ got_privsep_recv_tree(struct got_tree_object **tree, s
> if (err)
> break;
> }
> -done:
> +
> if (*tree && (*tree)->nentries != nentries) {
> if (err == NULL)
> err = got_error(GOT_ERR_PRIVSEP_LEN);
> @@ -2424,9 +2386,6 @@ got_privsep_recv_gotconfig_str(char **str, struct imsg
> datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
>
> switch (imsg.hdr.type) {
> - case GOT_IMSG_ERROR:
> - err = recv_imsg_error(&imsg, datalen);
> - break;
> case GOT_IMSG_GOTCONFIG_STR_VAL:
> if (datalen == 0)
> break;
> @@ -2470,9 +2429,6 @@ got_privsep_recv_gotconfig_remotes(struct got_remote_r
> datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
>
> switch (imsg.hdr.type) {
> - case GOT_IMSG_ERROR:
> - err = recv_imsg_error(&imsg, datalen);
> - break;
> case GOT_IMSG_GOTCONFIG_REMOTES:
> if (datalen != sizeof(iremotes)) {
> err = got_error(GOT_ERR_PRIVSEP_LEN);
> @@ -2511,9 +2467,6 @@ got_privsep_recv_gotconfig_remotes(struct got_remote_r
> datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
>
> switch (imsg.hdr.type) {
> - case GOT_IMSG_ERROR:
> - err = recv_imsg_error(&imsg, datalen);
> - break;
> case GOT_IMSG_GOTCONFIG_REMOTE:
> remote = &(*remotes)[*nremotes];
> memset(remote, 0, sizeof(*remote));
> blob - a48a4df951d4896c4fd29a461f3c17002b01bc3e
> blob + 73996be84412da8b6da8967a3b861c8e404900d5
> --- libexec/got-read-patch/got-read-patch.c
> +++ libexec/got-read-patch/got-read-patch.c
> @@ -92,9 +92,7 @@ send_patch_done(void)
> if (imsg_compose(&ibuf, GOT_IMSG_PATCH_DONE, 0, 0, -1,
> NULL, 0) == -1)
> return got_error_from_errno("imsg_compose GOT_IMSG_PATCH_EOF");
> - if (imsg_flush(&ibuf) == -1)
> - return got_error_from_errno("imsg_flush");
> - return NULL;
> + return got_privsep_flush_imsg(&ibuf);
> }
>
> /* based on fetchname from usr.bin/patch/util.c */
>
>
got_privsep_recv_tree tweaks and more cosmetic stuff