On Fri, Dec 18, 2020 at 05:17:51PM +0100, Christian Weisgerber wrote:
> Stefan Sperling:
> 
> > > What purpose does got_worktree_open() serve in the -r case?
> > > Maybe just skip got_worktree_open() completely, so worktree==NULL?
> > 
> > Yes, you're right. The worktree is opened in order to lock it, but
> > if we're never going to do anything with it, we might as well not
> > open it in the first place. Is this better?
> > 
> > diff refs/heads/main refs/heads/togrflag
> 
> Yes, that's what I had in mind.  I have only cosmetic comments:
> 
> * In cmd_diff(), got_worktree_open() can also move into the
>   "if (repo_path == NULL)" branch.
> 
> * Throughout, the preceding getcwd() can also move into the
>   "if (repo_path == NULL)" branch.

Right. Makes sense. Thanks!

I have just pushed this to the main branch:

-----------------------------------------------
commit c156c7a4f456c171f9e458793a2baa06389f8e1e (main, origin/main)
from: Stefan Sperling <stsp@stsp.name>
date: Fri Dec 18 19:19:46 2020 UTC
 
 do not mix up repositories if tog's -r option is used inside a work tree
 
 with input from and ok naddy
 
diff b9cc5cc45b0ff18a132212438eaf2d2f558166c7 22b7862a8fdd3e30a826a3c9dfeebd77dd532afe
blob - 334d9a659f385c9de036c79d21a1a527807feeb6
blob + d3d4a63398b4c4b05caa731b093b038b0b1d9bee
--- tog/tog.c
+++ tog/tog.c
@@ -2702,25 +2702,23 @@ cmd_log(int argc, char *argv[])
 	if (argc > 1)
 		usage_log();
 
-	cwd = getcwd(NULL, 0);
-	if (cwd == NULL)
-		return got_error_from_errno("getcwd");
-
-	error = got_worktree_open(&worktree, cwd);
-	if (error && error->code != GOT_ERR_NOT_WORKTREE)
-		goto done;
-
 	if (repo_path == NULL) {
+		cwd = getcwd(NULL, 0);
+		if (cwd == NULL)
+			return got_error_from_errno("getcwd");
+		error = got_worktree_open(&worktree, cwd);
+		if (error && error->code != GOT_ERR_NOT_WORKTREE)
+			goto done;
 		if (worktree)
 			repo_path =
 			    strdup(got_worktree_get_repo_path(worktree));
 		else
 			repo_path = strdup(cwd);
+		if (repo_path == NULL) {
+			error = got_error_from_errno("strdup");
+			goto done;
+		}
 	}
-	if (repo_path == NULL) {
-		error = got_error_from_errno("strdup");
-		goto done;
-	}
 
 	error = got_repo_open(&repo, repo_path, NULL);
 	if (error != NULL)
@@ -3789,25 +3787,23 @@ cmd_diff(int argc, char *argv[])
 	} else
 		usage_diff();
 
-	cwd = getcwd(NULL, 0);
-	if (cwd == NULL)
-		return got_error_from_errno("getcwd");
-
-	error = got_worktree_open(&worktree, cwd);
-	if (error && error->code != GOT_ERR_NOT_WORKTREE)
-		goto done;
-
 	if (repo_path == NULL) {
+		cwd = getcwd(NULL, 0);
+		if (cwd == NULL)
+			return got_error_from_errno("getcwd");
+		error = got_worktree_open(&worktree, cwd);
+		if (error && error->code != GOT_ERR_NOT_WORKTREE)
+			goto done;
 		if (worktree)
 			repo_path =
 			    strdup(got_worktree_get_repo_path(worktree));
 		else
 			repo_path = strdup(cwd);
+		if (repo_path == NULL) {
+			error = got_error_from_errno("strdup");
+			goto done;
+		}
 	}
-	if (repo_path == NULL) {
-		error = got_error_from_errno("strdup");
-		goto done;
-	}
 
 	error = got_repo_open(&repo, repo_path, NULL);
 	if (error)
@@ -4679,25 +4675,23 @@ cmd_blame(int argc, char *argv[])
 	if (argc != 1)
 		usage_blame();
 
-	cwd = getcwd(NULL, 0);
-	if (cwd == NULL)
-		return got_error_from_errno("getcwd");
-
-	error = got_worktree_open(&worktree, cwd);
-	if (error && error->code != GOT_ERR_NOT_WORKTREE)
-		goto done;
-
 	if (repo_path == NULL) {
+		cwd = getcwd(NULL, 0);
+		if (cwd == NULL)
+			return got_error_from_errno("getcwd");
+		error = got_worktree_open(&worktree, cwd);
+		if (error && error->code != GOT_ERR_NOT_WORKTREE)
+			goto done;
 		if (worktree)
 			repo_path =
 			    strdup(got_worktree_get_repo_path(worktree));
 		else
 			repo_path = strdup(cwd);
+		if (repo_path == NULL) {
+			error = got_error_from_errno("strdup");
+			goto done;
+		}
 	}
-	if (repo_path == NULL) {
-		error = got_error_from_errno("strdup");
-		goto done;
-	}
 
 	error = got_repo_open(&repo, repo_path, NULL);
 	if (error != NULL)
@@ -5491,25 +5485,23 @@ cmd_tree(int argc, char *argv[])
 	if (argc > 1)
 		usage_tree();
 
-	cwd = getcwd(NULL, 0);
-	if (cwd == NULL)
-		return got_error_from_errno("getcwd");
-
-	error = got_worktree_open(&worktree, cwd);
-	if (error && error->code != GOT_ERR_NOT_WORKTREE)
-		goto done;
-
 	if (repo_path == NULL) {
+		cwd = getcwd(NULL, 0);
+		if (cwd == NULL)
+			return got_error_from_errno("getcwd");
+		error = got_worktree_open(&worktree, cwd);
+		if (error && error->code != GOT_ERR_NOT_WORKTREE)
+			goto done;
 		if (worktree)
 			repo_path =
 			    strdup(got_worktree_get_repo_path(worktree));
 		else
 			repo_path = strdup(cwd);
+		if (repo_path == NULL) {
+			error = got_error_from_errno("strdup");
+			goto done;
+		}
 	}
-	if (repo_path == NULL) {
-		error = got_error_from_errno("strdup");
-		goto done;
-	}
 
 	error = got_repo_open(&repo, repo_path, NULL);
 	if (error != NULL)
@@ -6212,25 +6204,23 @@ cmd_ref(int argc, char *argv[])
 	if (argc > 1)
 		usage_ref();
 
-	cwd = getcwd(NULL, 0);
-	if (cwd == NULL)
-		return got_error_from_errno("getcwd");
-
-	error = got_worktree_open(&worktree, cwd);
-	if (error && error->code != GOT_ERR_NOT_WORKTREE)
-		goto done;
-
 	if (repo_path == NULL) {
+		cwd = getcwd(NULL, 0);
+		if (cwd == NULL)
+			return got_error_from_errno("getcwd");
+		error = got_worktree_open(&worktree, cwd);
+		if (error && error->code != GOT_ERR_NOT_WORKTREE)
+			goto done;
 		if (worktree)
 			repo_path =
 			    strdup(got_worktree_get_repo_path(worktree));
 		else
 			repo_path = strdup(cwd);
+		if (repo_path == NULL) {
+			error = got_error_from_errno("strdup");
+			goto done;
+		}
 	}
-	if (repo_path == NULL) {
-		error = got_error_from_errno("strdup");
-		goto done;
-	}
 
 	error = got_repo_open(&repo, repo_path, NULL);
 	if (error != NULL)