On Wed, Dec 16, 2020 at 10:53:03AM -0800, Yang Zhong wrote:
> Here's the patch using the format error function. I've resolved the smaller
> issues mentioned earlier as well.

Very nice, thank you! Ok.

> diff --git a/include/got_repository.h b/include/got_repository.h
> index 9e816e24..58d890d3 100644
> --- a/include/got_repository.h
> +++ b/include/got_repository.h
> @@ -32,6 +32,9 @@ const char *got_repo_get_path(struct got_repository *);
>   */
>  const char *got_repo_get_path_git_dir(struct got_repository *);
>  
> +/* Obtain the file descriptor of the repository's .git directory. */
> +int got_repo_get_fd(struct got_repository *);
> +
>  /* Obtain the commit author name if parsed from gitconfig, else NULL. */
>  const char *got_repo_get_gitconfig_author_name(struct got_repository *);
>  
> diff --git a/lib/got_lib_pack.h b/lib/got_lib_pack.h
> index c56e16c4..39aa4333 100644
> --- a/lib/got_lib_pack.h
> +++ b/lib/got_lib_pack.h
> @@ -168,7 +168,7 @@ struct got_packfile_obj_data {
>  
>  const struct got_error *got_packidx_init_hdr(struct got_packidx *, int);
>  const struct got_error *got_packidx_open(struct got_packidx **,
> -    const char *, int);
> +    int, const char *, int);
>  const struct got_error *got_packidx_close(struct got_packidx *);
>  int got_packidx_get_object_idx(struct got_packidx *, struct got_object_id *);
>  const struct got_error *got_packidx_match_id_str_prefix(
> diff --git a/lib/got_lib_repository.h b/lib/got_lib_repository.h
> index d2a6b78c..18d2953b 100644
> --- a/lib/got_lib_repository.h
> +++ b/lib/got_lib_repository.h
> @@ -34,6 +34,7 @@
>  struct got_repository {
>  	char *path;
>  	char *path_git_dir;
> +	int gitdir_fd;
>  
>  	/* The pack index cache speeds up search for packed objects. */
>  	struct got_packidx *packidx_cache[GOT_PACKIDX_CACHE_SIZE];
> diff --git a/lib/pack.c b/lib/pack.c
> index 3ce002a8..800248fb 100644
> --- a/lib/pack.c
> +++ b/lib/pack.c
> @@ -330,7 +330,8 @@ done:
>  }
>  
>  const struct got_error *
> -got_packidx_open(struct got_packidx **packidx, const char *path, int verify)
> +got_packidx_open(struct got_packidx **packidx,
> +    int dir_fd, const char *relpath, int verify)
>  {
>  	const struct got_error *err = NULL;
>  	struct got_packidx *p;
> @@ -342,15 +343,15 @@ got_packidx_open(struct got_packidx **packidx, const char *path, int verify)
>  	if (p == NULL)
>  		return got_error_from_errno("calloc");
>  
> -	p->fd = open(path, O_RDONLY | O_NOFOLLOW);
> +	p->fd = openat(dir_fd, relpath, O_RDONLY | O_NOFOLLOW);
>  	if (p->fd == -1) {
> -		err = got_error_from_errno2("open", path);
> +		err = got_error_from_errno2("openat", relpath);
>  		free(p);
>  		return err;
>  	}
>  
>  	if (fstat(p->fd, &sb) != 0) {
> -		err = got_error_from_errno2("fstat", path);
> +		err = got_error_from_errno2("fstat", relpath);
>  		close(p->fd);
>  		free(p);
>  		return err;
> @@ -363,7 +364,7 @@ got_packidx_open(struct got_packidx **packidx, const char *path, int verify)
>  		return err;
>  	}
>  
> -	p->path_packidx = strdup(path);
> +	p->path_packidx = strdup(relpath);
>  	if (p->path_packidx == NULL) {
>  		err = got_error_from_errno("strdup");
>  		goto done;
> diff --git a/lib/repository.c b/lib/repository.c
> index 02895008..729eb66d 100644
> --- a/lib/repository.c
> +++ b/lib/repository.c
> @@ -79,6 +79,12 @@ got_repo_get_path_git_dir(struct got_repository *repo)
>  	return repo->path_git_dir;
>  }
>  
> +int
> +got_repo_get_fd(struct got_repository *repo)
> +{
> +	return repo->gitdir_fd;
> +}
> +
>  const char *
>  got_repo_get_gitconfig_author_name(struct got_repository *repo)
>  {
> @@ -335,6 +341,8 @@ open_repo(struct got_repository *repo, const char *path)
>  {
>  	const struct got_error *err = NULL;
>  
> +	repo->gitdir_fd = -1;
> +
>  	/* bare git repository? */
>  	repo->path_git_dir = strdup(path);
>  	if (repo->path_git_dir == NULL)
> @@ -345,6 +353,12 @@ open_repo(struct got_repository *repo, const char *path)
>  			err = got_error_from_errno("strdup");
>  			goto done;
>  		}
> +		repo->gitdir_fd = open(repo->path_git_dir, O_DIRECTORY);
> +		if (repo->gitdir_fd == -1) {
> +			err = got_error_from_errno2("open",
> +			    repo->path_git_dir);
> +			goto done;
> +		}
>  		return NULL;
>  	}
>  
> @@ -361,6 +375,12 @@ open_repo(struct got_repository *repo, const char *path)
>  			err = got_error_from_errno("strdup");
>  			goto done;
>  		}
> +		repo->gitdir_fd = open(repo->path_git_dir, O_DIRECTORY);
> +		if (repo->gitdir_fd == -1) {
> +			err = got_error_from_errno2("open",
> +			    repo->path_git_dir);
> +			goto done;
> +		}
>  		return NULL;
>  	}
>  
> @@ -371,6 +391,10 @@ done:
>  		repo->path = NULL;
>  		free(repo->path_git_dir);
>  		repo->path_git_dir = NULL;
> +		if (repo->gitdir_fd != -1)
> +			close(repo->gitdir_fd);
> +		repo->gitdir_fd = -1;
> +
>  	}
>  	return err;
>  }
> @@ -918,11 +942,11 @@ got_repo_search_packidx(struct got_packidx **packidx, int *idx,
>      struct got_repository *repo, struct got_object_id *id)
>  {
>  	const struct got_error *err;
> -	char *path_packdir;
> -	DIR *packdir;
> +	DIR *packdir = NULL;
>  	struct dirent *dent;
>  	char *path_packidx;
>  	size_t i;
> +	int packdir_fd;
>  
>  	/* Search pack index cache. */
>  	for (i = 0; i < nitems(repo->packidx_cache); i++) {
> @@ -946,16 +970,21 @@ got_repo_search_packidx(struct got_packidx **packidx, int *idx,
>  	}
>  	/* No luck. Search the filesystem. */
>  
> -	path_packdir = got_repo_get_path_objects_pack(repo);
> -	if (path_packdir == NULL)
> -		return got_error_from_errno("got_repo_get_path_objects_pack");
> -
> -	packdir = opendir(path_packdir);
> -	if (packdir == NULL) {
> +	packdir_fd = openat(got_repo_get_fd(repo),
> +	    GOT_OBJECTS_PACK_DIR, O_DIRECTORY);
> +	if (packdir_fd == -1) {
>  		if (errno == ENOENT)
>  			err = got_error_no_obj(id);
>  		else
> -			err = got_error_from_errno2("opendir", path_packdir);
> +			err = got_error_from_errno_fmt("openat: %s/%s",
> +			    got_repo_get_path_git_dir(repo),
> +			    GOT_OBJECTS_PACK_DIR);
> +		goto done;
> +	}
> +
> +	packdir = fdopendir(packdir_fd);
> +	if (packdir == NULL) {
> +		err = got_error_from_errno("fdopendir");
>  		goto done;
>  	}
>  
> @@ -965,7 +994,7 @@ got_repo_search_packidx(struct got_packidx **packidx, int *idx,
>  		if (!is_packidx_filename(dent->d_name, dent->d_namlen))
>  			continue;
>  
> -		if (asprintf(&path_packidx, "%s/%s", path_packdir,
> +		if (asprintf(&path_packidx, "%s/%s", GOT_OBJECTS_PACK_DIR,
>  		    dent->d_name) == -1) {
>  			err = got_error_from_errno("asprintf");
>  			goto done;
> @@ -985,7 +1014,8 @@ got_repo_search_packidx(struct got_packidx **packidx, int *idx,
>  			continue; /* already searched */
>  		}
>  
> -		err = got_packidx_open(packidx, path_packidx, 0);
> +		err = got_packidx_open(packidx, got_repo_get_fd(repo),
> +		    path_packidx, 0);
>  		if (err) {
>  			free(path_packidx);
>  			goto done;
> @@ -1005,7 +1035,6 @@ got_repo_search_packidx(struct got_packidx **packidx, int *idx,
>  
>  	err = got_error_no_obj(id);
>  done:
> -	free(path_packdir);
>  	if (packdir && closedir(packdir) != 0 && err == NULL)
>  		err = got_error_from_errno("closedir");
>  	return err;
> @@ -1034,13 +1063,15 @@ read_packfile_hdr(int fd, struct got_packidx *packidx)
>  }
>  
>  static const struct got_error *
> -open_packfile(int *fd, const char *path_packfile, struct got_packidx *packidx)
> +open_packfile(int *fd, struct got_repository *repo,
> +    const char *relpath, struct got_packidx *packidx)
>  {
>  	const struct got_error *err = NULL;
>  
> -	*fd = open(path_packfile, O_RDONLY | O_NOFOLLOW);
> +	*fd = openat(got_repo_get_fd(repo), relpath, O_RDONLY | O_NOFOLLOW);
>  	if (*fd == -1)
> -		return got_error_from_errno2("open", path_packfile);
> +		return got_error_from_errno_fmt("openat: %s/%s",
> +		    got_repo_get_path_git_dir(repo), relpath);
>  
>  	if (packidx) {
>  		err = read_packfile_hdr(*fd, packidx);
> @@ -1090,7 +1121,7 @@ got_repo_cache_pack(struct got_pack **packp, struct got_repository *repo,
>  		goto done;
>  	}
>  
> -	err = open_packfile(&pack->fd, path_packfile, packidx);
> +	err = open_packfile(&pack->fd, repo, path_packfile, packidx);
>  	if (err)
>  		goto done;
>  
> @@ -1202,22 +1233,25 @@ match_packed_object(struct got_object_id **unique_id,
>      struct got_repository *repo, const char *id_str_prefix, int obj_type)
>  {
>  	const struct got_error *err = NULL;
> -	char *path_packdir;
> -	DIR *packdir;
> +	DIR *packdir = NULL;
>  	struct dirent *dent;
>  	char *path_packidx;
>  	struct got_object_id_queue matched_ids;
> +	int packdir_fd;
>  
>  	SIMPLEQ_INIT(&matched_ids);
>  
> -	path_packdir = got_repo_get_path_objects_pack(repo);
> -	if (path_packdir == NULL)
> -		return got_error_from_errno("got_repo_get_path_objects_pack");
> +	packdir_fd = openat(got_repo_get_fd(repo),
> +	    GOT_OBJECTS_PACK_DIR, O_DIRECTORY);
> +	if (packdir_fd == -1) {
> +		if (errno != ENOENT)
> +			err = got_error_from_errno2("openat", GOT_OBJECTS_PACK_DIR);
> +		goto done;
> +	}
>  
> -	packdir = opendir(path_packdir);
> +	packdir = fdopendir(packdir_fd);
>  	if (packdir == NULL) {
> -		if (errno != ENOENT)
> -			err = got_error_from_errno2("opendir", path_packdir);
> +		err = got_error_from_errno("fdopendir");
>  		goto done;
>  	}
>  
> @@ -1225,17 +1259,17 @@ match_packed_object(struct got_object_id **unique_id,
>  		struct got_packidx *packidx;
>  		struct got_object_qid *qid;
>  
> -
>  		if (!is_packidx_filename(dent->d_name, dent->d_namlen))
>  			continue;
>  
> -		if (asprintf(&path_packidx, "%s/%s", path_packdir,
> +		if (asprintf(&path_packidx, "%s/%s", GOT_OBJECTS_PACK_DIR,
>  		    dent->d_name) == -1) {
> -			err = got_error_from_errno("asprintf");
> +			err = got_error_from_errno("strdup");
>  			break;
>  		}
>  
> -		err = got_packidx_open(&packidx, path_packidx, 0);
> +		err = got_packidx_open(&packidx, got_repo_get_fd(repo),
> +		    path_packidx, 0);
>  		free(path_packidx);
>  		if (err)
>  			break;
> @@ -1276,7 +1310,6 @@ match_packed_object(struct got_object_id **unique_id,
>  	}
>  done:
>  	got_object_id_queue_free(&matched_ids);
> -	free(path_packdir);
>  	if (packdir && closedir(packdir) != 0 && err == NULL)
>  		err = got_error_from_errno("closedir");
>  	if (err) {