From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gitwrap: don't wait(), just execl()
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Tue, 28 Mar 2023 13:30:24 +0200

On Tue, Mar 28, 2023 at 12:48:45PM +0200, Omar Polo wrote:
> As a semplification, since we're going to exec(), we could directly do
> so without fork() + wait() in the main process.  So far I've only
> tested it from the shell (and hit an issue in
> got_serve_parse_command()) but it's easy to see that git-*-pack or
> gotsh are still correctly executed.
> 
> (we loose the _exit(127) but that was dead code anyway.)
> 
> ok?

Ah, so it was the "proc" pledge promise getting in the way of
just running exec()?

Thanks, it is working fine for me. ok

> diff /home/op/w/got
> commit - 33121ca5fbb746824ea01ae2f92e0200ab14207b
> path + /home/op/w/got
> blob - b76352f09eb64d8b4a8abdea88ec12556b121f49
> file + gitwrapper/gitwrapper.c
> --- gitwrapper/gitwrapper.c
> +++ gitwrapper/gitwrapper.c
> @@ -21,7 +21,6 @@
>  #include <sys/queue.h>
>  #include <sys/types.h>
>  #include <sys/uio.h>
> -#include <sys/wait.h>
>  
>  #include <err.h>
>  #include <errno.h>
> @@ -104,13 +103,11 @@ main(int argc, char *argv[])
>  	char *gitcommand = NULL;
>  	struct gotd gotd;
>  	struct gotd_repo *repo = NULL;
> -	pid_t pid;
> -	int st = -1;
>  
>  	log_init(1, LOG_USER); /* Log to stderr. */
>  
>  #ifndef PROFILE
> -	if (pledge("stdio rpath proc exec unveil", NULL) == -1)
> +	if (pledge("stdio rpath exec unveil", NULL) == -1)
>  		err(1, "pledge");
>  #endif
>  
> @@ -140,7 +137,7 @@ main(int argc, char *argv[])
>  		goto done;
>  
>  #ifndef PROFILE
> -	if (pledge("stdio proc exec", NULL) == -1)
> +	if (pledge("stdio exec", NULL) == -1)
>  		err(1, "pledge");
>  #endif
>  
> @@ -180,43 +177,25 @@ main(int argc, char *argv[])
>  	 * Invoke our custom Git server if the repository was found
>  	 * in gotd.conf. Otherwise invoke native git(1) tooling.
>  	 */
> -	switch (pid = fork()) {
> -	case -1:
> -		goto done;
> -	case 0:
> -		if (repo) {
> -			if (myserver == NULL) {
> -				error = got_error_fmt(GOT_ERR_NO_PROG,
> -				    "cannot run '%s'",
> -				    GITWRAPPER_MY_SERVER_PROG);
> -				goto done;
> -			}
> -			if (execl(myserver, command, repo_name,
> -			    (char *)NULL) ==  -1) {
> -				error = got_error_from_errno2("execl",
> -				    myserver);
> -				goto done;
> -			}
> -		} else {
> -			if (asprintf(&gitcommand, "%s/%s",
> -			    GITWRAPPER_GIT_LIBEXEC_DIR, command) == -1) {
> -				error = got_error_from_errno("asprintf");
> -				goto done;
> -			}
> -			if (execl(gitcommand, gitcommand, repo_path,
> -			    (char *)NULL) == -1) {
> -				error = got_error_from_errno2("execl",
> -				    gitcommand);
> -				goto done;
> -			}
> +	if (repo) {
> +		if (myserver == NULL) {
> +			error = got_error_fmt(GOT_ERR_NO_PROG,
> +			    "cannot run '%s'",
> +			    GITWRAPPER_MY_SERVER_PROG);
> +			goto done;
>  		}
> -		_exit(127);
> +		execl(myserver, command, repo_name, (char *)NULL);
> +		error = got_error_from_errno2("execl", myserver);
> +	} else {
> +		if (asprintf(&gitcommand, "%s/%s",
> +		    GITWRAPPER_GIT_LIBEXEC_DIR, command) == -1) {
> +			error = got_error_from_errno("asprintf");
> +			goto done;
> +		}
> +		execl(gitcommand, gitcommand, repo_path, (char *)NULL);
> +		error = got_error_from_errno2("execl", gitcommand);
>  	}
>  
> -	while (waitpid(pid, &st, 0) == -1) {
> -		if (errno != EINTR)
> -			break;
> -	}
>  done:
>  	free(command);
>  	free(repo_name);
> 
>