commit 1d0f405485b02cc4480ea188879e4122e0ea32bd
from: Stefan Sperling <stsp@stsp.name>
date: Thu Jun 17 08:56:43 2021 UTC

check for close(2) error in got_repo_close() and propagate errors up

ok tracey

commit - 991ff1aa4f423a1faea1bae0e85a913a88038309
commit + 1d0f405485b02cc4480ea188879e4122e0ea32bd
blob - 192b847bcb16c4c5fe215175d5a8109d0712f76b
blob + 097226da2568c25ff532d61bf72b41de086e3af9
--- got/got.c
+++ got/got.c
@@ -1800,8 +1800,11 @@ done:
 	}
 	if (fetchfd != -1 && close(fetchfd) == -1 && error == NULL)
 		error = got_error_from_errno("close");
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	TAILQ_FOREACH(pe, &refs, entry) {
 		free((void *)pe->path);
 		free(pe->data);
@@ -2515,8 +2518,11 @@ done:
 	}
 	if (fetchfd != -1 && close(fetchfd) == -1 && error == NULL)
 		error = got_error_from_errno("close");
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	TAILQ_FOREACH(pe, &refs, entry) {
@@ -4063,10 +4069,9 @@ done:
 	if (worktree)
 		got_worktree_close(worktree);
 	if (repo) {
-		const struct got_error *repo_error;
-		repo_error = got_repo_close(repo);
+		const struct got_error *close_err = got_repo_close(repo);
 		if (error == NULL)
-			error = repo_error;
+			error = close_err;
 	}
 	if (refs_idmap)
 		got_reflist_object_id_map_free(refs_idmap);
@@ -4488,10 +4493,9 @@ done:
 	if (worktree)
 		got_worktree_close(worktree);
 	if (repo) {
-		const struct got_error *repo_error;
-		repo_error = got_repo_close(repo);
+		const struct got_error *close_err = got_repo_close(repo);
 		if (error == NULL)
-			error = repo_error;
+			error = close_err;
 	}
 	got_ref_list_free(&refs);
 	return error;
@@ -4818,10 +4822,9 @@ done:
 	if (worktree)
 		got_worktree_close(worktree);
 	if (repo) {
-		const struct got_error *repo_error;
-		repo_error = got_repo_close(repo);
+		const struct got_error *close_err = got_repo_close(repo);
 		if (error == NULL)
-			error = repo_error;
+			error = close_err;
 	}
 	if (bca.lines) {
 		for (i = 0; i < bca.nlines; i++) {
@@ -5106,10 +5109,9 @@ done:
 	if (worktree)
 		got_worktree_close(worktree);
 	if (repo) {
-		const struct got_error *repo_error;
-		repo_error = got_repo_close(repo);
+		const struct got_error *close_err = got_repo_close(repo);
 		if (error == NULL)
-			error = repo_error;
+			error = close_err;
 	}
 	return error;
 }
@@ -5495,8 +5497,11 @@ cmd_ref(int argc, char *argv[])
 	}
 done:
 	free(refname);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	free(cwd);
@@ -5877,8 +5882,11 @@ cmd_branch(int argc, char *argv[])
 done:
 	if (ref)
 		got_ref_close(ref);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	free(cwd);
@@ -6387,8 +6395,11 @@ cmd_tag(int argc, char *argv[])
 		    commit_id_str ? commit_id_str : commit_id_arg, tagmsg);
 	}
 done:
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	free(cwd);
@@ -6519,8 +6530,11 @@ cmd_add(int argc, char *argv[])
 	error = got_worktree_schedule_add(worktree, &paths, add_progress,
 	    NULL, repo, no_ignores);
 done:
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	TAILQ_FOREACH(pe, &paths, entry)
@@ -6668,8 +6682,11 @@ cmd_remove(int argc, char *argv[])
 	    delete_local_mods, status_codes, print_remove_status, NULL,
 	    repo, keep_on_disk);
 done:
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	TAILQ_FOREACH(pe, &paths, entry)
@@ -6929,8 +6946,11 @@ done:
 	if (patch_script_file && fclose(patch_script_file) == EOF &&
 	    error == NULL)
 		error = got_error_from_errno2("fclose", patch_script_path);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	free(path);
@@ -7234,8 +7254,11 @@ done:
 	    error == NULL)
 		error = got_error_from_errno2("unlink", cl_arg.logmsg_path);
 	free(cl_arg.logmsg_path);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	free(cwd);
@@ -7365,8 +7388,11 @@ done:
 		got_ref_close(head_ref);
 	if (worktree)
 		got_worktree_close(worktree);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	return error;
 }
 
@@ -7485,8 +7511,11 @@ done:
 		got_ref_close(head_ref);
 	if (worktree)
 		got_worktree_close(worktree);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	return error;
 }
 
@@ -8322,8 +8351,11 @@ done:
 		got_ref_close(tmp_branch);
 	if (worktree)
 		got_worktree_close(worktree);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	return error;
 }
 
@@ -9620,8 +9652,11 @@ done:
 		got_ref_close(tmp_branch);
 	if (worktree)
 		got_worktree_close(worktree);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	return error;
 }
 
@@ -9753,8 +9788,11 @@ cmd_integrate(int argc, char *argv[])
 	printf("Integrated %s into %s\n", refname, base_refname);
 	print_update_progress_stats(&upa);
 done:
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	free(cwd);
@@ -9900,8 +9938,11 @@ done:
 	if (patch_script_file && fclose(patch_script_file) == EOF &&
 	    error == NULL)
 		error = got_error_from_errno2("fclose", patch_script_path);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	TAILQ_FOREACH(pe, &paths, entry)
@@ -10010,8 +10051,11 @@ done:
 	if (patch_script_file && fclose(patch_script_file) == EOF &&
 	    error == NULL)
 		error = got_error_from_errno2("fclose", patch_script_path);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	TAILQ_FOREACH(pe, &paths, entry)
@@ -10322,10 +10366,9 @@ done:
 	if (worktree)
 		got_worktree_close(worktree);
 	if (repo) {
-		const struct got_error *repo_error;
-		repo_error = got_repo_close(repo);
+		const struct got_error *close_err = got_repo_close(repo);
 		if (error == NULL)
-			error = repo_error;
+			error = close_err;
 	}
 	got_ref_list_free(&refs);
 	return error;
blob - 4cc5428af954c47772c98d07aff210d72929cc7c
blob + 3a689cd14ad0dacbec839cd0d7dead37f4d5f83c
--- gotweb/gotweb.c
+++ gotweb/gotweb.c
@@ -2833,8 +2833,11 @@ gw_get_repo_age(char **repo_age, struct gw_trans *gw_t
 	}
 done:
 	got_ref_list_free(&refs);
-	if (gw_trans->repo == NULL)
-		got_repo_close(repo);
+	if (gw_trans->repo == NULL) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	return error;
 }
 
@@ -2927,7 +2930,7 @@ done:
 static const struct got_error *
 gw_get_repo_owner(char **owner, struct gw_trans *gw_trans, char *dir)
 {
-	const struct got_error *error = NULL;
+	const struct got_error *error = NULL, *close_err;
 	struct got_repository *repo;
 	const char *gitconfig_owner;
 
@@ -2945,7 +2948,9 @@ gw_get_repo_owner(char **owner, struct gw_trans *gw_tr
 		if (*owner == NULL)
 			error = got_error_from_errno("strdup");
 	}
-	got_repo_close(repo);
+	close_err = got_repo_close(repo);
+	if (error == NULL)
+		error = close_err;
 	return error;
 }
 
@@ -4736,6 +4741,12 @@ main(int argc, char *argv[])
 	else
 		error = gw_display_index(gw_trans);
 done:
+	if (gw_trans->repo) {
+		const struct got_error *close_err;
+		close_err = got_repo_close(gw_trans->repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (error) {
 		gw_trans->error = error;
 		gw_trans->action = GW_ERR;
@@ -4768,9 +4779,6 @@ cleanup:
 	free(gw_trans->prev_id);
 	free(gw_trans->prev_prev_id);
 	free(gw_trans->repo_path);
-	if (gw_trans->repo)
-		got_repo_close(gw_trans->repo);
-
 	TAILQ_FOREACH_SAFE(dir, &gw_trans->gw_dirs, entry, tdir) {
 		free(dir->name);
 		free(dir->description);
blob - 16616cca614bcd144f1e84477ab65ea3f34b7b0e
blob + 2f1abf58087e2648591e03485c907e2b79bb7baa
--- lib/repository.c
+++ lib/repository.c
@@ -752,8 +752,9 @@ got_repo_close(struct got_repository *repo)
 			err = got_error_from_errno("close");
 	}
 
-	if (repo->gitdir_fd != -1)
-		close(repo->gitdir_fd);
+	if (repo->gitdir_fd != -1 && close(repo->gitdir_fd) == -1 &&
+	    err == NULL)
+		err = got_error_from_errno("close");
 
 	if (repo->gotconfig)
 		got_gotconfig_free(repo->gotconfig);
blob - 13cc9fbaca9c08dbb86084c978e908bee7eebe17
blob + 522c2fe12ca9c96345952c0586dcca8194d69202
--- lib/worktree.c
+++ lib/worktree.c
@@ -436,8 +436,11 @@ open_worktree(struct got_worktree **worktree, const ch
 		goto done;
 	}
 done:
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (err == NULL)
+			err = close_err;
+	}
 	free(path_got);
 	free(path_lock);
 	free(base_commit_id_str);
blob - c426e9d11b25926d3d473cea50910a6e8ecd3240
blob + b6840e939dec6d2d98e812c0a0ca34d74e03eee1
--- tog/tog.c
+++ tog/tog.c
@@ -2111,7 +2111,7 @@ stop_log_thread(struct tog_log_view_state *s)
 	}
 
 	if (s->thread_args.repo) {
-		got_repo_close(s->thread_args.repo);
+		err = got_repo_close(s->thread_args.repo);
 		s->thread_args.repo = NULL;
 	}
 
@@ -2806,8 +2806,11 @@ done:
 	free(label);
 	if (ref)
 		got_ref_close(ref);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	tog_free_refs();
@@ -3861,8 +3864,11 @@ done:
 	free(label2);
 	free(repo_path);
 	free(cwd);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	tog_free_refs();
@@ -4092,7 +4098,7 @@ done:
 static void *
 blame_thread(void *arg)
 {
-	const struct got_error *err;
+	const struct got_error *err, *close_err;
 	struct tog_blame_thread_args *ta = arg;
 	struct tog_blame_cb_args *a = ta->cb_args;
 	int errcode;
@@ -4111,7 +4117,9 @@ blame_thread(void *arg)
 		return (void *)got_error_set_errno(errcode,
 		    "pthread_mutex_lock");
 
-	got_repo_close(ta->repo);
+	close_err = got_repo_close(ta->repo);
+	if (err == NULL)
+		err = close_err;
 	ta->repo = NULL;
 	*ta->complete = 1;
 
@@ -4162,7 +4170,10 @@ stop_blame(struct tog_blame *blame)
 		blame->thread = NULL;
 	}
 	if (blame->thread_args.repo) {
-		got_repo_close(blame->thread_args.repo);
+		const struct got_error *close_err;
+		close_err = got_repo_close(blame->thread_args.repo);
+		if (err == NULL)
+			err = close_err;
 		blame->thread_args.repo = NULL;
 	}
 	if (blame->f) {
@@ -4782,8 +4793,11 @@ done:
 	free(commit_id);
 	if (worktree)
 		got_worktree_close(worktree);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	tog_free_refs();
 	return error;
 }
@@ -5613,8 +5627,11 @@ done:
 		got_object_commit_close(commit);
 	if (tree)
 		got_object_tree_close(tree);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	tog_free_refs();
 	return error;
 }
@@ -6289,8 +6306,11 @@ cmd_ref(int argc, char *argv[])
 done:
 	free(repo_path);
 	free(cwd);
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		const struct got_error *close_err = got_repo_close(repo);
+		if (close_err)
+			error = close_err;
+	}
 	tog_free_refs();
 	return error;
 }
@@ -6354,7 +6374,7 @@ make_argv(int argc, ...)
 static const struct got_error *
 tog_log_with_path(int argc, char *argv[])
 {
-	const struct got_error *error = NULL;
+	const struct got_error *error = NULL, *close_err;
 	struct tog_cmd *cmd = NULL;
 	struct got_repository *repo = NULL;
 	struct got_worktree *worktree = NULL;
@@ -6412,7 +6432,9 @@ tog_log_with_path(int argc, char *argv[])
 		/* not reached */
 	}
 
-	got_repo_close(repo);
+	close_err = got_repo_close(repo);
+	if (error == NULL)
+		error = close_err;
 	repo = NULL;
 
 	error = got_object_id_str(&commit_id_str, commit_id);
@@ -6424,8 +6446,11 @@ tog_log_with_path(int argc, char *argv[])
 	cmd_argv = make_argv(argc, cmd->name, "-c", commit_id_str, argv[0]);
 	error = cmd->cmd_main(argc, cmd_argv);
 done:
-	if (repo)
-		got_repo_close(repo);
+	if (repo) {
+		close_err = got_repo_close(repo);
+		if (error == NULL)
+			error = close_err;
+	}
 	if (worktree)
 		got_worktree_close(worktree);
 	free(id);