On 23-04-14 02:06PM, Stefan Sperling wrote:
> On Thu, Apr 13, 2023 at 06:34:05PM +1000, Mark Jamsek wrote:
> > On 23-04-13 12:01AM, Mark Jamsek wrote:
> > > Yes, we can definitely open it before calling pledge(). I was about to
> > > zzz so will send this now but will cook a diff tomorrow to see if that
> > > fixes the pledge issue. Thanks :)
> > 
> > Unfortunately, my initial suspicions were valid: opening the screendump
> > file before pledging still doesn't work. I thought you might be onto
> > something because earlier I was calling init_mock_term() from main
> > before pledge() and it was aborting--but I wasn't opening the screendump
> > file then.
> > 
> > I checked-out that revision and moved the screendump file opening to
> > init_mock_term() before pledge, but no dice.
> > 
> > It seems every curses ioctl() request SIGABRTs. When we init_curses(),
> > we uncook the terminal with cbreak(), which calls ioctl():
> 
> The reason for the aborts is that pledge will not permit ioctls
> related to TTYs on non-tty files. Which makes sense.
> 
> I do believe we should run tests under pledge if at all possible.
> Because otherwise our tests will not catch code which introduces
> pledge violations.

Yes, definitely! I think tog regress should run under pledge.

> The patch below makes the existing tog tests pass with pledge active.
> What do you think?

Very nice, ok. I was actually working on a similar diff with minor
changes but I like this better; I was also calling init_curses() from
main. I've added a patch below with one of the differences though, which
applies on top of yours. It just consolidates all the tog_io_close()
calls at the end of main().

We may also need to make future changes as test coverage grows but this
is great having regress pledged. Thank you!

diff /home/mark/src/got
commit - 324ddda19751f233f3f99b1dba4e950e955d8ae1
path + /home/mark/src/got
blob - 45cbc985724f5499495edf465cbddf52611fbae8
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -4189,10 +4189,7 @@ init_mock_term(const char *test_script_path)
 		goto done;
 	}
 
-	/*
-	 * XXX Perhaps we should define "xterm" as the terminal
-	 * type for standardised testing instead of using $TERM?
-	 */
+	/* use local TERM so we test in different environments */
 	if (newterm(NULL, tog_io.cout, tog_io.cin) == NULL)
 		err = got_error_msg(GOT_ERR_IO,
 		    "newterm: failed to initialise curses");
@@ -4274,7 +4271,7 @@ cmd_log(int argc, char *argv[])
 static const struct got_error *
 cmd_log(int argc, char *argv[])
 {
-	const struct got_error *io_err, *error;
+	const struct got_error *error;
 	struct got_repository *repo = NULL;
 	struct got_worktree *worktree = NULL;
 	struct got_object_id *start_id = NULL;
@@ -4412,11 +4409,6 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	if (using_mock_io) {
-		io_err = tog_io_close();
-		if (error == NULL)
-			error = io_err;
-	}
 	tog_free_refs();
 	return error;
 }
@@ -5786,7 +5778,7 @@ cmd_diff(int argc, char *argv[])
 static const struct got_error *
 cmd_diff(int argc, char *argv[])
 {
-	const struct got_error *io_err, *error;
+	const struct got_error *error;
 	struct got_repository *repo = NULL;
 	struct got_worktree *worktree = NULL;
 	struct got_object_id *id1 = NULL, *id2 = NULL;
@@ -5912,11 +5904,6 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	if (using_mock_io) {
-		io_err = tog_io_close();
-		if (error == NULL)
-			error = io_err;
-	}
 	tog_free_refs();
 	return error;
 }
@@ -6891,7 +6878,7 @@ cmd_blame(int argc, char *argv[])
 static const struct got_error *
 cmd_blame(int argc, char *argv[])
 {
-	const struct got_error *io_err, *error;
+	const struct got_error *error;
 	struct got_repository *repo = NULL;
 	struct got_worktree *worktree = NULL;
 	char *cwd = NULL, *repo_path = NULL, *in_repo_path = NULL;
@@ -7028,11 +7015,6 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	if (using_mock_io) {
-		io_err = tog_io_close();
-		if (error == NULL)
-			error = io_err;
-	}
 	tog_free_refs();
 	return error;
 }
@@ -7865,7 +7847,7 @@ cmd_tree(int argc, char *argv[])
 static const struct got_error *
 cmd_tree(int argc, char *argv[])
 {
-	const struct got_error *io_err, *error;
+	const struct got_error *error;
 	struct got_repository *repo = NULL;
 	struct got_worktree *worktree = NULL;
 	char *cwd = NULL, *repo_path = NULL, *in_repo_path = NULL;
@@ -8005,11 +7987,6 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	if (using_mock_io) {
-		io_err = tog_io_close();
-		if (error == NULL)
-			error = io_err;
-	}
 	tog_free_refs();
 	return error;
 }
@@ -8757,7 +8734,7 @@ cmd_ref(int argc, char *argv[])
 static const struct got_error *
 cmd_ref(int argc, char *argv[])
 {
-	const struct got_error *io_err, *error;
+	const struct got_error *error;
 	struct got_repository *repo = NULL;
 	struct got_worktree *worktree = NULL;
 	char *cwd = NULL, *repo_path = NULL;
@@ -8851,11 +8828,6 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	if (using_mock_io) {
-		io_err = tog_io_close();
-		if (error == NULL)
-			error = io_err;
-	}
 	tog_free_refs();
 	return error;
 }
@@ -9722,7 +9694,7 @@ main(int argc, char *argv[])
 int
 main(int argc, char *argv[])
 {
-	const struct got_error *error = NULL;
+	const struct got_error *io_err, *error = NULL;
 	const struct tog_cmd *cmd = NULL;
 	int ch, hflag = 0, Vflag = 0;
 	char **cmd_argv = NULL;
@@ -9821,6 +9793,11 @@ main(int argc, char *argv[])
 			error = cmd->cmd_main(argc, cmd_argv ? cmd_argv : argv);
 	}
 
+	if (using_mock_io) {
+		io_err = tog_io_close();
+		if (error == NULL)
+			error = io_err;
+	}
 	endwin();
 	if (cmd_argv) {
 		int i;

-- 
Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68