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

From:
Omar Polo <op@omarpolo.com>
Subject:
got_privsep_recv_tree tweaks and more cosmetic stuff
To:
gameoftrees@openbsd.org
Date:
Thu, 07 Jul 2022 13:24:20 +0200

Download raw body.

Thread
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.

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 */