From: Omar Polo <op@omarpolo.com>
Subject: Re: gotd: defer some checks while starting
To: Omar Polo <op@omarpolo.com>
Cc: Stefan Sperling <stsp@stsp.name>, gameoftrees@openbsd.org
Date: Tue, 17 Jan 2023 18:55:08 +0100

On 2023/01/17 17:43:25 +0100, Omar Polo <op@omarpolo.com> wrote:
> since i was there, what about moving some checks inside parse_config?
> 
> it slightly improves the error message if `listen on socket' is not an
> absolute path.

i can't send a diff on the first try today it seems.

after walking away from the keyboard i thought that having the
got_path_is_absolute check only in the listen process was a waste: if
the value is wrong, report it directly from the main process without
waiting to spawning the listener.  It's also nicer because it's caught
at `gotd -n' time:

	% cat /etc/gotd.conf
	user op
	listen on 'gotd.sock'
	repository 'ports' {
	        path '/home/op/git/ports.git'
	        permit ro gotdev
	}
	% doas gotd -n
	configuration OK
	% make && doas ./obj/gotd -n
	gotd: /etc/gotd.conf:2: bad unix socket path "gotd.sock": \
	    must be an absolute path

ok?

diff /tmp/got
commit - f4e8c21cb2dc6a468bae32a4dcf3a9e18f269527
path + /tmp/got
blob - 070e37a9d1342993266222f1483916a70450a206
file + gotd/gotd.c
--- gotd/gotd.c
+++ gotd/gotd.c
@@ -1764,10 +1764,6 @@ main(int argc, char **argv)
 	if (parse_config(confpath, proc_id, &gotd) != 0)
 		return 1;
 
-	if (proc_id == PROC_GOTD &&
-	    (gotd.nrepos == 0 || TAILQ_EMPTY(&gotd.repos)))
-		fatalx("no repository defined in configuration file");
-
 	pw = getpwnam(gotd.user_name);
 	if (pw == NULL)
 		fatalx("user %s not found", gotd.user_name);
@@ -1775,11 +1771,6 @@ main(int argc, char **argv)
 	if (pw->pw_uid == 0)
 		fatalx("cannot run %s as the superuser", getprogname());
 
-	if (proc_id == PROC_LISTEN &&
-	    !got_path_is_absolute(gotd.unix_socket_path))
-		fatalx("bad unix socket path \"%s\": must be an absolute path",
-		    gotd.unix_socket_path);
-
 	if (noaction) {
 		fprintf(stderr, "configuration OK\n");
 		return 0;
blob - 9be73f985cd4b495f70ba089cee603ee8afc861d
file + gotd/parse.y
--- gotd/parse.y
+++ gotd/parse.y
@@ -195,6 +195,10 @@ main		: LISTEN ON STRING {
 		;
 
 main		: LISTEN ON STRING {
+			if (!got_path_is_absolute($3))
+				yyerror("bad unix socket path \"%s\": "
+				    "must be an absolute path", $3);
+
 			if (gotd_proc_id == PROC_LISTEN) {
 				if (strlcpy(gotd->unix_socket_path, $3,
 				    sizeof(gotd->unix_socket_path)) >=
@@ -744,6 +748,11 @@ parse_config(const char *filename, enum gotd_procid pr
 		}
 	}
 
+	if (proc_id == PROC_GOTD && TAILQ_EMPTY(&gotd->repos)) {
+		log_warnx("no repository defined in configuration file");
+		return (-1);
+	}
+
 	return (0);
 }