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

On Tue, Jan 17, 2023 at 04:15:14PM +0100, Omar Polo wrote:
> a slightly better diff.  it's equivalent to the previous but by moving
> the block where we set gotd.argv = argv0 below too makes easier to see
> that it gets assigned to 'valid' data.

Seems fine to me. ok.
 
> diff /tmp/got
> commit - f08fa56da9b67c42d726230b936ee664a5ce0e42
> path + /tmp/got
> blob - 717b4729fac0259057de6bd02cadff694a709e40
> file + gotd/gotd.c
> --- gotd/gotd.c
> +++ gotd/gotd.c
> @@ -1758,24 +1758,12 @@ 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;
>  
> -	gotd.argv0 = argv0;
> -	gotd.daemonize = daemonize;
> -	gotd.verbosity = verbosity;
> -	gotd.confpath = confpath;
> -
>  	if (proc_id == PROC_GOTD &&
>  	    (gotd.nrepos == 0 || TAILQ_EMPTY(&gotd.repos)))
>  		fatalx("no repository defined in configuration file");
> @@ -1784,20 +1772,31 @@ 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;
> +	}
>  
> +	gotd.argv0 = argv0;
> +	gotd.daemonize = daemonize;
> +	gotd.verbosity = verbosity;
> +	gotd.confpath = confpath;
> +
> +	/* 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]);
> 
> 
>