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

From:
Omar Polo <op@omarpolo.com>
Subject:
gotd: defer some checks while starting
To:
gameoftrees@openbsd.org
Date:
Tue, 17 Jan 2023 15:27:51 +0100

Download raw body.

Thread
I'm a bit afraid to send this diff right after 0.80 has been tagged,
but I've noticed this stuff only now that I was playing with the rc
script.  (since now i've only ever run gotd from the command line with
-v.)

The absolute path check is quite annoying when one just wants to see
if the configuration is fine (i often do this manually.)  log_init
could also be called a bit later, so that we always show the error for
the configuration:

before the diff

	% doas /usr/local/sbin/gotd -n
	% echo $?
	1
	% doas /usr/local/sbin/gotd -n -d
	gotd: /etc/gotd.conf:1: syntax error

after:

	% make && doas ./gotd -n
	gotd: /etc/gotd.conf:1: syntax error

While here I was tempted to also print a message if the configuration
is OK (like httpd does for example) and to tweak an error message:

% doas /usr/local/sbin/gotd -d -n
gotd: : cannot run gotd as root: the user running gotd must not be the superuser
% make && doas ./gotd -n
gotd: : cannot run gotd as the superuser

ok?

diff /tmp/got
commit - f08fa56da9b67c42d726230b936ee664a5ce0e42
path + /tmp/got
blob - 717b4729fac0259057de6bd02cadff694a709e40
file + gotd/gotd.c
--- gotd/gotd.c
+++ gotd/gotd.c
@@ -1758,16 +1758,9 @@ main(int argc, char **argv)
 	if (argc != 0)
 		usage();
 
-	/* Require an absolute path in argv[0] for reliable re-exec. */
-	if (!got_path_is_absolute(argv0))
-		fatalx("bad path \"%s\": must be an absolute path", argv0);
-
 	if (geteuid() && (proc_id == PROC_GOTD || proc_id == PROC_LISTEN))
 		fatalx("need root privileges");
 
-	log_init(daemonize ? 0 : 1, LOG_DAEMON);
-	log_setverbose(verbosity);
-
 	if (parse_config(confpath, proc_id, &gotd) != 0)
 		return 1;
 
@@ -1784,20 +1777,26 @@ main(int argc, char **argv)
 	if (pw == NULL)
 		fatalx("user %s not found", gotd.user_name);
 
-	if (pw->pw_uid == 0) {
-		fatalx("cannot run %s as %s: the user running %s "
-		    "must not be the superuser",
-		    getprogname(), pw->pw_name, getprogname());
-	}
+	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)
+	if (noaction) {
+		fprintf(stderr, "configuration OK\n");
 		return 0;
+	}
 
+	/* Require an absolute path in argv[0] for reliable re-exec. */
+	if (!got_path_is_absolute(argv0))
+		fatalx("bad path \"%s\": must be an absolute path", argv0);
+
+	log_init(daemonize ? 0 : 1, LOG_DAEMON);
+	log_setverbose(verbosity);
+
 	if (proc_id == PROC_GOTD) {
 		gotd.pid = getpid();
 		snprintf(title, sizeof(title), "%s", gotd_proc_names[proc_id]);