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

From:
Omar Polo <op@omarpolo.com>
Subject:
chores; move patch send/recv to privsep.c
To:
gameoftrees@openbsd.org
Date:
Mon, 04 Jul 2022 22:27:13 +0200

Download raw body.

Thread
'got patch' is the only part of got that sends and receives messages
outside of privsep.c.  When i first implemented it I haven't realize
this.

I tried to minimize the changes as much as possible.  The only
difference is that recv_patch (now got_privsep_recv_patch) doesn't call
patch_free anymore, as i didn't want to export that function too.
Instead, we explicitly call it in the error path in patch.c, not a big
deal.  I flipped the order of the params for consistency with the other
got_privsep_* functions.

thoughts/ok?


P.S. some of the errors used in got_privsep_recv_patch are not the best
choice, I intend to change them after this goes in.

diff /home/op/w/got
commit - 02a5c5d00338c9549f6a399391841bd8219d91cf
path + /home/op/w/got
blob - /dev/null
file + lib/got_lib_patch.h
--- /dev/null
+++ lib/got_lib_patch.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2022 Omar Polo <op@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+struct got_patch_hunk {
+	STAILQ_ENTRY(got_patch_hunk) entries;
+	const struct got_error *err;
+	int	ws_mangled;
+	int	offset;
+	int	old_nonl;
+	int	new_nonl;
+	int	old_from;
+	int	old_lines;
+	int	new_from;
+	int	new_lines;
+	size_t	len;
+	size_t	cap;
+	char	**lines;
+};
+
+STAILQ_HEAD(got_patch_hunk_head, got_patch_hunk);
+struct got_patch {
+	char	*old;
+	char	*new;
+	char	 cid[41];
+	char	 blob[41];
+	struct got_patch_hunk_head head;
+};
blob - dac4ab973b68243e262fd1ae6482fffb6dc2bc57
file + lib/got_lib_privsep.h
--- lib/got_lib_privsep.h
+++ lib/got_lib_privsep.h
@@ -660,6 +660,7 @@ struct got_remote_repo;
 struct got_pack;
 struct got_packidx;
 struct got_pathlist_head;
+struct got_patch;
 
 const struct got_error *got_send_ack(pid_t);
 const struct got_error *got_privsep_wait_for_child(pid_t);
@@ -827,4 +828,8 @@ const struct got_error *got_privsep_recv_painted_commi
     struct got_object_id_queue *, got_privsep_recv_painted_commit_cb, void *,
     struct imsgbuf *);
 
+const struct got_error *got_privsep_send_patch(int, struct imsgbuf *);
+const struct got_error *got_privsep_recv_patch(int *, struct got_patch *,
+    int, struct imsgbuf *);
+
 void got_privsep_exec_child(int[2], const char *, const char *);
blob - dbbc9b38387c388c49654f106cab82c723f3dd06
file + lib/patch.c
--- lib/patch.c
+++ lib/patch.c
@@ -53,61 +53,16 @@
 #include "got_lib_object.h"
 #include "got_lib_privsep.h"
 #include "got_lib_sha1.h"
+#include "got_lib_patch.h"
 
 #define MIN(a, b) ((a) < (b) ? (a) : (b))
 
-struct got_patch_hunk {
-	STAILQ_ENTRY(got_patch_hunk) entries;
-	const struct got_error *err;
-	int	ws_mangled;
-	int	offset;
-	int	old_nonl;
-	int	new_nonl;
-	int	old_from;
-	int	old_lines;
-	int	new_from;
-	int	new_lines;
-	size_t	len;
-	size_t	cap;
-	char	**lines;
-};
-
-STAILQ_HEAD(got_patch_hunk_head, got_patch_hunk);
-struct got_patch {
-	char	*old;
-	char	*new;
-	char	 cid[41];
-	char	 blob[41];
-	struct got_patch_hunk_head head;
-};
-
 struct patch_args {
 	got_patch_progress_cb progress_cb;
 	void	*progress_arg;
 	struct got_patch_hunk_head *head;
 };
 
-static const struct got_error *
-send_patch(struct imsgbuf *ibuf, int fd)
-{
-	const struct got_error *err = NULL;
-
-	if (imsg_compose(ibuf, GOT_IMSG_PATCH_FILE, 0, 0, fd,
-	    NULL, 0) == -1) {
-		err = got_error_from_errno(
-		    "imsg_compose GOT_IMSG_PATCH_FILE");
-		close(fd);
-		return err;
-	}
-
-	if (imsg_flush(ibuf) == -1) {
-		err = got_error_from_errno("imsg_flush");
-		imsg_clear(ibuf);
-	}
-
-	return err;
-}
-
 static void
 patch_free(struct got_patch *p)
 {
@@ -131,194 +86,6 @@ patch_free(struct got_patch *p)
 	STAILQ_INIT(&p->head);
 }
 
-static const struct got_error *
-pushline(struct got_patch_hunk *h, const char *line)
-{
-	void 	*t;
-	size_t	 newcap;
-
-	if (h->len == h->cap) {
-		if ((newcap = h->cap * 1.5) == 0)
-			newcap = 16;
-		t = recallocarray(h->lines, h->cap, newcap,
-		    sizeof(h->lines[0]));
-		if (t == NULL)
-			return got_error_from_errno("recallocarray");
-		h->lines = t;
-		h->cap = newcap;
-	}
-
-	if ((t = strdup(line)) == NULL)
-		return got_error_from_errno("strdup");
-
-	h->lines[h->len++] = t;
-	return NULL;
-}
-
-static const struct got_error *
-recv_patch(struct imsgbuf *ibuf, int *done, struct got_patch *p, int strip)
-{
-	const struct got_error *err = NULL;
-	struct imsg imsg;
-	struct got_imsg_patch_hunk hdr;
-	struct got_imsg_patch patch;
-	struct got_patch_hunk *h = NULL;
-	size_t datalen;
-	int lastmode = -1;
-
-	memset(p, 0, sizeof(*p));
-	STAILQ_INIT(&p->head);
-
-	err = got_privsep_recv_imsg(&imsg, ibuf, 0);
-	if (err)
-		return err;
-	if (imsg.hdr.type == GOT_IMSG_PATCH_EOF) {
-		*done = 1;
-		goto done;
-	}
-	if (imsg.hdr.type != GOT_IMSG_PATCH) {
-		err = got_error(GOT_ERR_PRIVSEP_MSG);
-		goto done;
-	}
-	datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
-	if (datalen != sizeof(patch)) {
-		err = got_error(GOT_ERR_PRIVSEP_LEN);
-		goto done;
-	}
-	memcpy(&patch, imsg.data, sizeof(patch));
-
-	if (patch.old[sizeof(patch.old)-1] != '\0' ||
-	    patch.new[sizeof(patch.new)-1] != '\0' ||
-	    patch.cid[sizeof(patch.cid)-1] != '\0' ||
-	    patch.blob[sizeof(patch.blob)-1] != '\0') {
-		err = got_error(GOT_ERR_PRIVSEP_LEN);
-		goto done;
-	}
-
-	if (*patch.cid != '\0')
-		strlcpy(p->cid, patch.cid, sizeof(p->cid));
-
-	if (*patch.blob != '\0')
-		strlcpy(p->blob, patch.blob, sizeof(p->blob));
-
-	/* automatically set strip=1 for git-style diffs */
-	if (strip == -1 && patch.git &&
-	    (*patch.old == '\0' || !strncmp(patch.old, "a/", 2)) &&
-	    (*patch.new == '\0' || !strncmp(patch.new, "b/", 2)))
-		strip = 1;
-
-	/* prefer the new name if not /dev/null for not git-style diffs */
-	if (!patch.git && *patch.new != '\0' && *patch.old != '\0') {
-		err = got_path_strip(&p->old, patch.new, strip);
-		if (err)
-			goto done;
-	} else if (*patch.old != '\0') {
-		err = got_path_strip(&p->old, patch.old, strip);
-		if (err)
-			goto done;
-	}
-
-	if (*patch.new != '\0') {
-		err = got_path_strip(&p->new, patch.new, strip);
-		if (err)
-			goto done;
-	}
-
-	if (p->old == NULL && p->new == NULL) {
-		err = got_error(GOT_ERR_PATCH_MALFORMED);
-		goto done;
-	}
-
-	imsg_free(&imsg);
-
-	for (;;) {
-		char *t;
-
-		err = got_privsep_recv_imsg(&imsg, ibuf, 0);
-		if (err) {
-			patch_free(p);
-			return err;
-		}
-
-		datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
-		switch (imsg.hdr.type) {
-		case GOT_IMSG_PATCH_DONE:
-			if (h != NULL && h->len == 0)
-				err = got_error(GOT_ERR_PATCH_MALFORMED);
-			goto done;
-		case GOT_IMSG_PATCH_HUNK:
-			if (h != NULL &&
-			    (h->len == 0 || h->old_nonl || h->new_nonl)) {
-				err = got_error(GOT_ERR_PATCH_MALFORMED);
-				goto done;
-			}
-			lastmode = -1;
-			if (datalen != sizeof(hdr)) {
-				err = got_error(GOT_ERR_PRIVSEP_LEN);
-				goto done;
-			}
-			memcpy(&hdr, imsg.data, sizeof(hdr));
-			if (hdr.oldfrom < 0 || hdr.newfrom < 0) {
-				err = got_error(GOT_ERR_PRIVSEP_LEN);
-				goto done;
-			}
-			if ((h = calloc(1, sizeof(*h))) == NULL) {
-				err = got_error_from_errno("calloc");
-				goto done;
-			}
-			h->old_from = hdr.oldfrom;
-			h->old_lines = hdr.oldlines;
-			h->new_from = hdr.newfrom;
-			h->new_lines = hdr.newlines;
-			STAILQ_INSERT_TAIL(&p->head, h, entries);
-			break;
-		case GOT_IMSG_PATCH_LINE:
-			if (h == NULL) {
-				err = got_error(GOT_ERR_PRIVSEP_MSG);
-				goto done;
-			}
-			t = imsg.data;
-			/* at least one char */
-			if (datalen < 2 || t[datalen-1] != '\0') {
-				err = got_error(GOT_ERR_PRIVSEP_MSG);
-				goto done;
-			}
-			if (*t != ' ' && *t != '-' && *t != '+' &&
-			    *t != '\\') {
-				err = got_error(GOT_ERR_PRIVSEP_MSG);
-				goto done;
-			}
-
-			if (*t != '\\')
-				err = pushline(h, t);
-			else if (lastmode == '-')
-				h->old_nonl = 1;
-			else if (lastmode == '+')
-				h->new_nonl = 1;
-			else
-				err = got_error(GOT_ERR_PATCH_MALFORMED);
-
-			if (err)
-				goto done;
-
-			lastmode = *t;
-			break;
-		default:
-			err = got_error(GOT_ERR_PRIVSEP_MSG);
-			goto done;
-		}
-
-		imsg_free(&imsg);
-	}
-
-done:
-	if (err)
-		patch_free(p);
-
-	imsg_free(&imsg);
-	return err;
-}
-
 /*
  * Copy data from orig starting at copypos until pos into tmp.
  * If pos is -1, copy until EOF.
@@ -985,7 +752,7 @@ got_patch(int fd, struct got_worktree *worktree, struc
 	imsg_fds[1] = -1;
 	imsg_init(ibuf, imsg_fds[0]);
 
-	err = send_patch(ibuf, fd);
+	err = got_privsep_send_patch(fd, ibuf);
 	fd = -1;
 	if (err)
 		goto done;
@@ -1003,9 +770,11 @@ got_patch(int fd, struct got_worktree *worktree, struc
 		pa.progress_arg = progress_arg;
 		pa.head = &p.head;
 
-		err = recv_patch(ibuf, &done, &p, strip);
-		if (err || done)
+		err = got_privsep_recv_patch(&done, &p, strip, ibuf);
+		if (err || done) {
+			patch_free(&p);
 			break;
+		}
 
 		if (reverse)
 			reverse_patch(&p);
blob - 9a16d647b870eb31f58c9d131d1174c60e7d5eb1
file + lib/privsep.c
--- lib/privsep.c
+++ lib/privsep.c
@@ -47,6 +47,7 @@
 #include "got_lib_object_parse.h"
 #include "got_lib_privsep.h"
 #include "got_lib_pack.h"
+#include "got_lib_patch.h"
 
 #include "got_privsep.h"
 
@@ -3527,6 +3528,212 @@ got_privsep_recv_painted_commits(struct got_object_id_
 }
 
 const struct got_error *
+got_privsep_send_patch(int fd, struct imsgbuf *ibuf)
+{
+	const struct got_error *err = NULL;
+
+	if (imsg_compose(ibuf, GOT_IMSG_PATCH_FILE, 0, 0, fd,
+	    NULL, 0) == -1) {
+		err = got_error_from_errno(
+		    "imsg_compose GOT_IMSG_PATCH_FILE");
+		close(fd);
+		return err;
+	}
+
+	if (imsg_flush(ibuf) == -1) {
+		err = got_error_from_errno("imsg_flush");
+		imsg_clear(ibuf);
+	}
+
+	return err;
+}
+
+static const struct got_error *
+patch_addline(struct got_patch_hunk *h, const char *line)
+{
+	void	*t;
+	size_t	 newcap;
+
+	if (h->len == h->cap) {
+		if ((newcap = h->cap * 1.5) == 0)
+			newcap = 16;
+		t = recallocarray(h->lines, h->cap, newcap,
+		    sizeof(h->lines[0]));
+		if (t == NULL)
+			return got_error_from_errno("recallocarray");
+		h->lines = t;
+		h->cap = newcap;
+	}
+
+	if ((t = strdup(line)) == NULL)
+		return got_error_from_errno("strdup");
+
+	h->lines[h->len++] = t;
+	return NULL;
+}
+
+const struct got_error *
+got_privsep_recv_patch(int *done, struct got_patch *p, int strip,
+    struct imsgbuf *ibuf)
+{
+	const struct got_error *err = NULL;
+	struct imsg imsg;
+	struct got_imsg_patch_hunk hdr;
+	struct got_imsg_patch patch;
+	struct got_patch_hunk *h = NULL;
+	size_t datalen;
+	int lastmode = -1;
+
+	*done = 0;
+	memset(p, 0, sizeof(*p));
+	STAILQ_INIT(&p->head);
+
+	err = got_privsep_recv_imsg(&imsg, ibuf, 0);
+	if (err)
+		return err;
+	if (imsg.hdr.type == GOT_IMSG_PATCH_EOF) {
+		*done = 1;
+		goto done;
+	}
+	if (imsg.hdr.type != GOT_IMSG_PATCH) {
+		err = got_error(GOT_ERR_PRIVSEP_MSG);
+		goto done;
+	}
+	datalen = imsg.hdr.len  - IMSG_HEADER_SIZE;
+	if (datalen != sizeof(patch)) {
+		err = got_error(GOT_ERR_PRIVSEP_LEN);
+		goto done;
+	}
+	memcpy(&patch, imsg.data, sizeof(patch));
+
+	if (patch.old[sizeof(patch.old) - 1] != '\0' ||
+	    patch.new[sizeof(patch.new) - 1] != '\0' ||
+	    patch.cid[sizeof(patch.cid) - 1] != '\0' ||
+	    patch.blob[sizeof(patch.blob) - 1] != '\0') {
+		err = got_error(GOT_ERR_PRIVSEP_LEN);
+		goto done;
+	}
+
+	if (*patch.cid != '\0')
+		strlcpy(p->cid, patch.cid, sizeof(p->cid));
+
+	if (*patch.blob != '\0')
+		strlcpy(p->blob, patch.blob, sizeof(p->blob));
+
+	/* automatically set strip=1 for git-style diffs */
+	if (strip == -1 && patch.git &&
+	    (*patch.old == '\0' || !strncmp(patch.old, "a/", 2)) &&
+	    (*patch.new == '\0' || !strncmp(patch.new, "b/", 2)))
+		strip = 1;
+
+	/* prefer the new name if not /dev/null for not git-style diffs */
+	if (!patch.git && *patch.new != '\0' && *patch.old != '\0') {
+		err = got_path_strip(&p->old, patch.new, strip);
+		if (err)
+			goto done;
+	} else if (*patch.old != '\0') {
+		err = got_path_strip(&p->old, patch.old, strip);
+		if (err)
+			goto done;
+	}
+
+	if (*patch.new != '\0') {
+		err = got_path_strip(&p->new, patch.new, strip);
+		if (err)
+			goto done;
+	}
+
+	if (p->old == NULL && p->new == NULL) {
+		err = got_error(GOT_ERR_PATCH_MALFORMED);
+		goto done;
+	}
+
+	imsg_free(&imsg);
+
+	for (;;) {
+		char *t;
+
+		err = got_privsep_recv_imsg(&imsg, ibuf, 0);
+		if (err)
+			return err;
+
+		datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
+		switch (imsg.hdr.type) {
+		case GOT_IMSG_PATCH_DONE:
+			if (h != NULL && h->len == 0)
+				err = got_error(GOT_ERR_PATCH_MALFORMED);
+			goto done;
+		case GOT_IMSG_PATCH_HUNK:
+			if (h != NULL &&
+			    (h->len == 0 || h->old_nonl || h->new_nonl)) {
+				err = got_error(GOT_ERR_PATCH_MALFORMED);
+				goto done;
+			}
+			lastmode = -1;
+			if (datalen != sizeof(hdr)) {
+				err = got_error(GOT_ERR_PRIVSEP_LEN);
+				goto done;
+			}
+			memcpy(&hdr, imsg.data, sizeof(hdr));
+			if (hdr.oldfrom < 0 || hdr.newfrom < 0) {
+				err = got_error(GOT_ERR_PRIVSEP_LEN);
+				goto done;
+			}
+			if ((h = calloc(1, sizeof(*h))) == NULL) {
+				err = got_error_from_errno("calloc");
+				goto done;
+			}
+			h->old_from = hdr.oldfrom;
+			h->old_lines = hdr.oldlines;
+			h->new_from = hdr.newfrom;
+			h->new_lines = hdr.newlines;
+			STAILQ_INSERT_TAIL(&p->head, h, entries);
+			break;
+		case GOT_IMSG_PATCH_LINE:
+			if (h == NULL) {
+				err = got_error(GOT_ERR_PRIVSEP_MSG);
+				goto done;
+			}
+			t = imsg.data;
+			/* at least one char */
+			if (datalen < 2 || t[datalen-1] != '\0') {
+				err = got_error(GOT_ERR_PRIVSEP_MSG);
+				goto done;
+			}
+			if (*t != ' ' && *t != '-' && *t != '+' &&
+			    *t != '\\') {
+				err = got_error(GOT_ERR_PRIVSEP_MSG);
+				goto done;
+			}
+
+			if (*t != '\\')
+				err = patch_addline(h, t);
+			else if (lastmode == '-')
+				h->old_nonl = 1;
+			else if (lastmode == '+')
+				h->new_nonl = 1;
+			else
+				err = got_error(GOT_ERR_PATCH_MALFORMED);
+
+			if (err)
+				goto done;
+
+			lastmode = *t;
+			break;
+		default:
+			err = got_error(GOT_ERR_PRIVSEP_MSG);
+			goto done;
+		}
+
+		imsg_free(&imsg);
+	}
+
+done:
+	imsg_free(&imsg);
+	return err;
+}
+
+const struct got_error *
 got_privsep_unveil_exec_helpers(void)
 {
 	const char *helpers[] = {