On Tue, Sep 05, 2023 at 01:22:20PM +0200, op@omarpolo.com wrote:
> 	 Test failures:
> 
> test failed; leaving test data in /home/chiaki/tmp/got-test-fetch_gotconfig_remote_repo-xd8FF9BttX

This is a crash which was introduced by my recent commit to release
the work tree lock earlier during send/fetch operations.

Fix below, ok?

-----------------------------------------------
 copy remote repo info out of work tree data before closing the work tree
 
 Fixes a crash regression introduced when fetch/send were made to close
 the work tree earlier.
 
diff 48d13ac6dbfcb3d6edc18b5d373009c0cec558fa bdc3493c16c8f04f767fe39fd2014e7cc81401eb
commit - 48d13ac6dbfcb3d6edc18b5d373009c0cec558fa
commit + bdc3493c16c8f04f767fe39fd2014e7cc81401eb
blob - 63a9ace21208033f773d2710814090a45e1cd079
blob + 73812ddbb0f2e3a0fee927fc0ae5b873c04e385a
--- got/got.c
+++ got/got.c
@@ -2288,7 +2288,8 @@ cmd_fetch(int argc, char *argv[])
 	const char *remote_name;
 	char *proto = NULL, *host = NULL, *port = NULL;
 	char *repo_name = NULL, *server_path = NULL;
-	const struct got_remote_repo *remotes, *remote = NULL;
+	const struct got_remote_repo *remotes;
+	struct got_remote_repo *remote = NULL;
 	int nremotes;
 	char *id_str = NULL;
 	struct got_repository *repo = NULL;
@@ -2450,7 +2451,10 @@ cmd_fetch(int argc, char *argv[])
 			    worktree_conf);
 			for (i = 0; i < nremotes; i++) {
 				if (strcmp(remotes[i].name, remote_name) == 0) {
-					remote = &remotes[i];
+					error = got_repo_remote_repo_dup(&remote,
+					    &remotes[i]);
+					if (error)
+						goto done;
 					break;
 				}
 			}
@@ -2463,7 +2467,10 @@ cmd_fetch(int argc, char *argv[])
 			    repo_conf);
 			for (i = 0; i < nremotes; i++) {
 				if (strcmp(remotes[i].name, remote_name) == 0) {
-					remote = &remotes[i];
+					error = got_repo_remote_repo_dup(&remote,
+					    &remotes[i]);
+					if (error)
+						goto done;
 					break;
 				}
 			}
@@ -2473,7 +2480,10 @@ cmd_fetch(int argc, char *argv[])
 		got_repo_get_gitconfig_remotes(&nremotes, &remotes, repo);
 		for (i = 0; i < nremotes; i++) {
 			if (strcmp(remotes[i].name, remote_name) == 0) {
-				remote = &remotes[i];
+				error = got_repo_remote_repo_dup(&remote,
+				    &remotes[i]);
+				if (error)
+					goto done;
 				break;
 			}
 		}
@@ -2796,6 +2806,8 @@ done:
 	got_pathlist_free(&wanted_branches, GOT_PATHLIST_FREE_NONE);
 	got_pathlist_free(&wanted_refs, GOT_PATHLIST_FREE_NONE);
 	got_ref_list_free(&remote_refs);
+	got_repo_free_remote_repo_data(remote);
+	free(remote);
 	free(head_refname);
 	free(id_str);
 	free(cwd);
@@ -9693,7 +9705,8 @@ cmd_send(int argc, char *argv[])
 	const char *remote_name;
 	char *proto = NULL, *host = NULL, *port = NULL;
 	char *repo_name = NULL, *server_path = NULL;
-	const struct got_remote_repo *remotes, *remote = NULL;
+	const struct got_remote_repo *remotes;
+	struct got_remote_repo *remote = NULL;
 	int nremotes, nbranches = 0, ndelete_branches = 0;
 	struct got_repository *repo = NULL;
 	struct got_worktree *worktree = NULL;
@@ -9828,7 +9841,10 @@ cmd_send(int argc, char *argv[])
 			    worktree_conf);
 			for (i = 0; i < nremotes; i++) {
 				if (strcmp(remotes[i].name, remote_name) == 0) {
-					remote = &remotes[i];
+					error = got_repo_remote_repo_dup(&remote,
+					    &remotes[i]);
+					if (error)
+						goto done;
 					break;
 				}
 			}
@@ -9841,7 +9857,10 @@ cmd_send(int argc, char *argv[])
 			    repo_conf);
 			for (i = 0; i < nremotes; i++) {
 				if (strcmp(remotes[i].name, remote_name) == 0) {
-					remote = &remotes[i];
+					error = got_repo_remote_repo_dup(&remote,
+					    &remotes[i]);
+					if (error)
+						goto done;
 					break;
 				}
 			}
@@ -9851,7 +9870,10 @@ cmd_send(int argc, char *argv[])
 		got_repo_get_gitconfig_remotes(&nremotes, &remotes, repo);
 		for (i = 0; i < nremotes; i++) {
 			if (strcmp(remotes[i].name, remote_name) == 0) {
-				remote = &remotes[i];
+				error = got_repo_remote_repo_dup(&remote,
+				    &remotes[i]);
+				if (error)
+					goto done;
 				break;
 			}
 		}
@@ -10041,6 +10063,8 @@ done:
 	}
 	if (ref)
 		got_ref_close(ref);
+	got_repo_free_remote_repo_data(remote);
+	free(remote);
 	got_pathlist_free(&branches, GOT_PATHLIST_FREE_NONE);
 	got_pathlist_free(&tags, GOT_PATHLIST_FREE_NONE);
 	got_ref_list_free(&all_branches);
blob - 17dcc926d4cae7fa2f169eb003ed4aeebba92346
blob + c53bf9434590b858c1ce59018954e81ff934dfc1
--- include/got_repository.h
+++ include/got_repository.h
@@ -88,6 +88,14 @@ struct got_remote_repo {
 };
 
 /*
+ * Return a deep copy of a given remote_repo. The result should be
+ * freed with got_repo_free_remote_repo_data() and then free(3).
+ * Return NULL on failure.
+ */
+const struct got_error *got_repo_remote_repo_dup(struct got_remote_repo **,
+    const struct got_remote_repo *);
+
+/*
  * Free data allocated for the specified remote repository.
  * Do not free the remote_repo pointer itself.
  */
blob - 4e71a4e99ffaf5422e72a6b4039c70bc6a837630
blob + c428c4539fe5daebf60dc208642df9cafeb4a217
--- lib/repository.c
+++ lib/repository.c
@@ -907,11 +907,115 @@ got_repo_close(struct got_repository *repo)
 	return err;
 }
 
+const struct got_error *
+got_repo_remote_repo_dup(struct got_remote_repo **newp,
+    const struct got_remote_repo *repo)
+{
+	const struct got_error *err = NULL;
+	struct got_remote_repo *new;
+	int i;
+
+	new = calloc(1, sizeof(*new));
+	if (new == NULL)
+		return got_error_from_errno("calloc");
+	
+	if (repo->name) {
+		new->name = strdup(repo->name);
+		if (new->name == NULL) {
+			err = got_error_from_errno("strdup");
+			goto done;
+		}
+	}
+
+	if (repo->fetch_url) {
+		new->fetch_url = strdup(repo->fetch_url);
+		if (new->fetch_url == NULL) {
+			err = got_error_from_errno("strdup");
+			goto done;
+		}
+	}
+
+	if (repo->send_url) {
+		new->send_url = strdup(repo->send_url);
+		if (new->send_url == NULL) {
+			err = got_error_from_errno("strdup");
+			goto done;
+		}
+	}
+
+	new->mirror_references = repo->mirror_references;
+
+	new->nfetch_branches = repo->nfetch_branches;
+	if (repo->fetch_branches) {
+		new->fetch_branches = calloc(repo->nfetch_branches,
+		    sizeof(char *));
+		if (new->fetch_branches == NULL) {
+			err = got_error_from_errno("calloc");
+			goto done;
+		}
+		for (i = 0; i < repo->nfetch_branches; i++) {
+			new->fetch_branches[i] = strdup(
+			    repo->fetch_branches[i]);
+			if (new->fetch_branches[i] == NULL) {
+				err = got_error_from_errno("strdup");
+				goto done;
+			}
+		}
+	}
+
+	new->nsend_branches = repo->nsend_branches;
+	if (repo->send_branches) {
+		new->send_branches = calloc(repo->nsend_branches,
+		    sizeof(char *));
+		if (new->send_branches == NULL) {
+			err = got_error_from_errno("calloc");
+			goto done;
+		}
+		for (i = 0; i < repo->nsend_branches; i++) {
+			new->send_branches[i] = strdup(
+			    repo->send_branches[i]);
+			if (new->send_branches[i] == NULL) {
+				err = got_error_from_errno("strdup");
+				goto done;
+			}
+		}
+	}
+
+	new->nfetch_refs = repo->nfetch_refs;
+	if (repo->fetch_refs) {
+		new->fetch_refs = calloc(repo->nfetch_refs,
+		    sizeof(char *));
+		if (new->fetch_refs == NULL) {
+			err = got_error_from_errno("calloc");
+			goto done;
+		}
+		for (i = 0; i < repo->nfetch_refs; i++) {
+			new->fetch_refs[i] = strdup(
+			    repo->fetch_refs[i]);
+			if (new->fetch_refs[i] == NULL) {
+				err = got_error_from_errno("strdup");
+				goto done;
+			}
+		}
+	}
+done:
+	if (err) {
+		got_repo_free_remote_repo_data(new);
+		free(new);
+	} else
+		*newp = new;
+
+	return err;
+}
+
 void
 got_repo_free_remote_repo_data(struct got_remote_repo *repo)
 {
 	int i;
 
+	if (repo == NULL)
+		return;
+
 	free(repo->name);
 	repo->name = NULL;
 	free(repo->fetch_url);
@@ -928,6 +1032,11 @@ got_repo_free_remote_repo_data(struct got_remote_repo 
 	free(repo->send_branches);
 	repo->send_branches = NULL;
 	repo->nsend_branches = 0;
+	for (i = 0; i < repo->nfetch_refs; i++)
+		free(repo->fetch_refs[i]);
+	free(repo->fetch_refs);
+	repo->fetch_refs = NULL;
+	repo->nfetch_refs = 0;
 }
 
 const struct got_error *