From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotwebd: simplify gotweb_load_got_path
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Thu, 1 Sep 2022 10:48:11 +0200

On Thu, Sep 01, 2022 at 10:40:39AM +0200, Omar Polo wrote:
> On 2022/09/01 09:44:14 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> > I think the check for GOT_DIR can be dropped. If someone runs gotwebd
> > on a work-tree then gotwebd will fail soon enough in some way. There is
> > no reason to treat this case with a special check.
> 
> agreed, here's a rebased and updated diff that also gc GOTWEB_GOT_DIR
> (now unused.)

Great, ok stsp

> diff refs/heads/main refs/heads/gwd-load-path
> commit - b5c757f5f816a8061f4879da9e68a39141148e40
> commit + 7407d6b52b8783876ff8ce04e314aafe1c55736a
> blob - 7520e71fd4327bf9eb5df008f47f25845a44564e
> blob + 06b0e06e2d7ebf7f9abe95e13bf22731f0389295
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -1979,7 +1979,6 @@ gotweb_load_got_path(struct request *c, struct repo_di
>  	struct got_repository *repo = NULL;
>  	DIR *dt;
>  	char *dir_test;
> -	int opened = 0;
>  
>  	if (asprintf(&dir_test, "%s/%s/%s", srv->repos_path, repo_dir->name,
>  	    GOTWEB_GIT_DIR) == -1)
> @@ -1989,52 +1988,24 @@ gotweb_load_got_path(struct request *c, struct repo_di
>  	if (dt == NULL) {
>  		free(dir_test);
>  	} else {
> -		repo_dir->path = strdup(dir_test);
> -		if (repo_dir->path == NULL) {
> -			opened = 1;
> -			error = got_error_from_errno("strdup");
> -			goto err;
> -		}
> -		opened = 1;
> -		goto done;
> -	}
> -
> -	if (asprintf(&dir_test, "%s/%s/%s", srv->repos_path, repo_dir->name,
> -	    GOTWEB_GOT_DIR) == -1) {
> +		repo_dir->path = dir_test;
>  		dir_test = NULL;
> -		error = got_error_from_errno("asprintf");
> -		goto err;
> +		goto done;
>  	}
>  
> -	dt = opendir(dir_test);
> -	if (dt == NULL)
> -		free(dir_test);
> -	else {
> -		opened = 1;
> -		error = got_error(GOT_ERR_NOT_GIT_REPO);
> -		goto err;
> -	}
> -
>  	if (asprintf(&dir_test, "%s/%s", srv->repos_path,
> -	    repo_dir->name) == -1) {
> -		error = got_error_from_errno("asprintf");
> -		dir_test = NULL;
> -		goto err;
> -	}
> +	    repo_dir->name) == -1)
> +		return got_error_from_errno("asprintf");
>  
> -	repo_dir->path = strdup(dir_test);
> -	if (repo_dir->path == NULL) {
> -		opened = 1;
> -		error = got_error_from_errno("strdup");
> -		goto err;
> -	}
> -
>  	dt = opendir(dir_test);
>  	if (dt == NULL) {
>  		error = got_error_path(repo_dir->name, GOT_ERR_NOT_GIT_REPO);
>  		goto err;
> -	} else
> -		opened = 1;
> +	} else {
> +		repo_dir->path = dir_test;
> +		dir_test = NULL;
> +	}
> +
>  done:
>  	repo = find_cached_repo(srv, repo_dir->path);
>  	if (repo == NULL) {
> @@ -2057,9 +2028,8 @@ done:
>  	error = gotweb_get_clone_url(&repo_dir->url, srv, repo_dir->path);
>  err:
>  	free(dir_test);
> -	if (opened)
> -		if (dt != NULL && closedir(dt) == EOF && error == NULL)
> -			error = got_error_from_errno("closedir");
> +	if (dt != NULL && closedir(dt) == EOF && error == NULL)
> +		error = got_error_from_errno("closedir");
>  	return error;
>  }
>  
> blob - 74ee67ae6734f6af0af3ab58f3276ceeccc8863a
> blob + bf762e12dd70fa07bb83c88d023e1ebb29d35f27
> --- gotwebd/gotwebd.h
> +++ gotwebd/gotwebd.h
> @@ -53,7 +53,6 @@
>  #define MAX_SCRIPT_NAME		 255
>  #define MAX_SERVER_NAME		 255
>  
> -#define GOTWEB_GOT_DIR		 ".got"
>  #define GOTWEB_GIT_DIR		 ".git"
>  
>  #define D_HTTPD_CHROOT		 "/var/www"
> 
>