This patch should make got and tog work with non-const dirname(3).

Note that got_path_dirname() treats input which does not contain a
slash as an error. I don't think this can happen in any of these
cases since they all seem to involve absolute paths only. And most
of the old code woulnd not deal well with dirname(3) returning "."

But I could be wrong and I don't know whether the test suite is
good enough to catch this. Any issues that pop up in the wild will
be easy enough to fix though. So I am not very worried about this.

Some minor unrelated bug fixes:

Replace a 'break' with 'goto err' in got_repo_open() in an error case.

Switch path comparisons from strcmp() to got_path_cmp() in order to
treat paths such as "foo//bar" and "foo/bar" as equal.

ok?

diff refs/heads/main refs/heads/dirname
blob - 89384c49f08bc3027b3bab5aea7dd94029669839
blob + 0219d3d35f26f8c729484c3c1e4243c3d1f8b97a
--- include/got_path.h
+++ include/got_path.h
@@ -101,7 +101,10 @@ const struct got_error *got_path_mkdir(const char *);
 /* Determine whether a directory has no files or directories in it. */
 int got_path_dir_is_empty(const char *);
 
-/* dirname(3) with error handling and dynamically allocated result. */
+/*
+ * dirname(3) with error handling, dynamically allocated result, and
+ * unmodified input.
+ */
 const struct got_error *got_path_dirname(char **, const char *);
 
 /*
blob - 0648aebafecc757f25710300b9e98d53c91c585a
blob + 54f8f6f973385072b6bba9a07e98cca11948d8b8
--- lib/path.c
+++ lib/path.c
@@ -358,9 +358,13 @@ got_path_dir_is_empty(const char *dir)
 const struct got_error *
 got_path_dirname(char **parent, const char *path)
 {
+	char buf[PATH_MAX];
 	char *p;
 
-	p = dirname(path);
+	if (strlcpy(buf, path, sizeof(buf)) >= sizeof(buf))
+		return got_error(GOT_ERR_NO_SPACE);
+
+	p = dirname(buf);
 	if (p == NULL)
 		return got_error_from_errno2("dirname", path);
 
blob - 5fe931f9c74dc53f1a6814a342b8eefc7d80f439
blob + 036305e904556fd6910f4e907610f8973a3b2e8a
--- lib/repository.c
+++ lib/repository.c
@@ -551,8 +551,8 @@ got_repo_open(struct got_repository **repop, const cha
 {
 	struct got_repository *repo = NULL;
 	const struct got_error *err = NULL;
-	char *abspath;
-	int i, tried_root = 0;
+	char *abspath, *repo_path = NULL;
+	int i;
 
 	*repop = NULL;
 
@@ -592,31 +592,30 @@ got_repo_open(struct got_repository **repop, const cha
 	if (err)
 		goto done;
 
-	path = realpath(abspath, NULL);
-	if (path == NULL) {
+	repo_path = realpath(abspath, NULL);
+	if (repo_path == NULL) {
 		err = got_error_from_errno2("realpath", abspath);
 		goto done;
 	}
 
-	do {
-		err = open_repo(repo, path);
+	for (;;) {
+		char *parent_path;
+
+		err = open_repo(repo, repo_path);
 		if (err == NULL)
 			break;
 		if (err->code != GOT_ERR_NOT_GIT_REPO)
-			break;
-		if (path[0] == '/' && path[1] == '\0') {
-			if (tried_root) {
-				err = got_error(GOT_ERR_NOT_GIT_REPO);
-				goto done;
-			}
-			tried_root = 1;
-		}
-		path = dirname(path);
-		if (path == NULL) {
-			err = got_error_from_errno2("dirname", path);
 			goto done;
+		if (repo_path[0] == '/' && repo_path[1] == '\0') {
+			err = got_error(GOT_ERR_NOT_GIT_REPO);
+			goto done;
 		}
-	} while (path);
+		err = got_path_dirname(&parent_path, repo_path);
+		if (err)
+			goto done;
+		free(repo_path);
+		repo_path = parent_path;
+	}
 
 	err = read_gotconfig(repo);
 	if (err)
@@ -633,6 +632,7 @@ done:
 	else
 		*repop = repo;
 	free(abspath);
+	free(repo_path);
 	return err;
 }
 
blob - ce156d2479af0056f65897680e9e32f8864800b6
blob + a46191b6be513b0aa32c2a166157fe86ee15aefe
--- lib/worktree.c
+++ lib/worktree.c
@@ -453,18 +453,39 @@ const struct got_error *
 got_worktree_open(struct got_worktree **worktree, const char *path)
 {
 	const struct got_error *err = NULL;
+	char *worktree_path;
 
-	do {
-		err = open_worktree(worktree, path);
-		if (err && !(err->code == GOT_ERR_ERRNO && errno == ENOENT))
+	worktree_path = strdup(path);
+	if (worktree_path == NULL)
+		return got_error_from_errno("strdup");
+
+	for (;;) {
+		char *parent_path;
+
+		err = open_worktree(worktree, worktree_path);
+		if (err && !(err->code == GOT_ERR_ERRNO && errno == ENOENT)) {
+			free(worktree_path);
 			return err;
-		if (*worktree)
+		}
+		if (*worktree) {
+			free(worktree_path);
 			return NULL;
-		path = dirname(path);
-		if (path == NULL)
-			return got_error_from_errno2("dirname", path);
-	} while (!((path[0] == '.' || path[0] == '/') && path[1] == '\0'));
+		}
+		if (worktree_path[0] == '/' && worktree_path[1] == '\0')
+			break;
+		err = got_path_dirname(&parent_path, worktree_path);
+		if (err) {
+			if (err->code != GOT_ERR_BAD_PATH) {
+				free(worktree_path);
+				return err;
+			}
+			break;
+		}
+		free(worktree_path);
+		worktree_path = parent_path;
+	}
 
+	free(worktree_path);
 	return got_error(GOT_ERR_NOT_WORKTREE);
 }
 
@@ -761,18 +782,20 @@ merge_file(int *local_changes_subsumed, struct got_wor
 	char *blob_orig_path = NULL;
 	char *merged_path = NULL, *base_path = NULL;
 	int overlapcnt = 0;
-	char *parent;
+	char *parent = NULL;
 	char *symlink_path = NULL;
 	FILE *symlinkf = NULL;
 
 	*local_changes_subsumed = 0;
 
-	parent = dirname(ondisk_path);
-	if (parent == NULL)
-		return got_error_from_errno2("dirname", ondisk_path);
+	err = got_path_dirname(&parent, ondisk_path);
+	if (err)
+		return err;
 
-	if (asprintf(&base_path, "%s/got-merged", parent) == -1)
-		return got_error_from_errno("asprintf");
+	if (asprintf(&base_path, "%s/got-merged", parent) == -1) {
+		err = got_error_from_errno("asprintf");
+		goto done;
+	}
 
 	err = got_opentemp_named_fd(&merged_path, &merged_fd, base_path);
 	if (err)
@@ -893,6 +916,7 @@ done:
 		unlink(blob_orig_path);
 		free(blob_orig_path);
 	}
+	free(parent);
 	return err;
 }
 
@@ -1100,13 +1124,13 @@ merge_blob(int *local_changes_subsumed, struct got_wor
 	const struct got_error *err = NULL;
 	FILE *f_deriv = NULL;
 	char *blob_deriv_path = NULL, *base_path = NULL, *id_str = NULL;
-	char *label_deriv = NULL, *parent;
+	char *label_deriv = NULL, *parent = NULL;
 
 	*local_changes_subsumed = 0;
 
-	parent = dirname(ondisk_path);
-	if (parent == NULL)
-		return got_error_from_errno2("dirname", ondisk_path);
+	err = got_path_dirname(&parent, ondisk_path);
+	if (err)
+		return err;
 
 	free(base_path);
 	if (asprintf(&base_path, "%s/got-merge-blob-deriv", parent) == -1) {
@@ -1145,6 +1169,7 @@ done:
 	}
 	free(id_str);
 	free(label_deriv);
+	free(parent);
 	return err;
 }
 
@@ -1267,12 +1292,15 @@ is_bad_symlink_target(int *is_bad_symlink, const char 
 	 * in which the blob object is being installed.
 	 */
 	if (!got_path_is_absolute(target_path)) {
-		char *abspath;
-		char *parent = dirname(ondisk_path);
-		if (parent == NULL)
-			return got_error_from_errno2("dirname", ondisk_path);
-		if (asprintf(&abspath, "%s/%s",  parent, target_path) == -1)
+		char *abspath, *parent;
+		err = got_path_dirname(&parent, ondisk_path);
+		if (err)
+			return err;
+		if (asprintf(&abspath, "%s/%s",  parent, target_path) == -1) {
+			free(parent);
 			return got_error_from_errno("asprintf");
+		}
+		free(parent);
 		if (strlen(abspath) >= sizeof(canonpath)) {
 			err = got_error_path(abspath, GOT_ERR_BAD_PATH);
 			free(abspath);
@@ -1386,13 +1414,12 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 		}
 
 		if (errno == ENOENT) {
-			char *parent = dirname(ondisk_path);
-			if (parent == NULL) {
-				err = got_error_from_errno2("dirname",
-				    ondisk_path);
+			char *parent;
+			err = got_path_dirname(&parent, ondisk_path);
+			if (err)
 				goto done;
-			}
 			err = add_dir_on_disk(worktree, parent);
+			free(parent);
 			if (err)
 				goto done;
 			/*
@@ -1448,10 +1475,12 @@ install_blob(struct got_worktree *worktree, const char
 	    GOT_DEFAULT_FILE_MODE);
 	if (fd == -1) {
 		if (errno == ENOENT) {
-			char *parent = dirname(path);
-			if (parent == NULL)
-				return got_error_from_errno2("dirname", path);
+			char *parent;
+			err = got_path_dirname(&parent, path);
+			if (err)
+				return err;
 			err = add_dir_on_disk(worktree, parent);
+			free(parent);
 			if (err)
 				return err;
 			fd = open(ondisk_path,
@@ -2018,16 +2047,22 @@ remove_ondisk_file(const char *root_path, const char *
 		if (errno != ENOENT)
 			err = got_error_from_errno2("unlink", ondisk_path);
 	} else {
-		char *parent = dirname(ondisk_path);
-		while (parent && strcmp(parent, root_path) != 0) {
-			if (rmdir(parent) == -1) {
+		size_t root_len = strlen(root_path);
+		do {
+			char *parent;
+			err = got_path_dirname(&parent, ondisk_path);
+			if (err)
+				return err;
+			free(ondisk_path);
+			ondisk_path = parent;
+			if (rmdir(ondisk_path) == -1) {
 				if (errno != ENOTEMPTY)
 					err = got_error_from_errno2("rmdir",
-					    parent);
+					    ondisk_path);
 				break;
 			}
-			parent = dirname(parent);
-		}
+		} while (got_path_cmp(ondisk_path, root_path,
+		    strlen(ondisk_path), root_len) != 0);
 	}
 	free(ondisk_path);
 	return err;
@@ -3807,7 +3842,7 @@ schedule_for_deletion(void *arg, unsigned char status,
 	const struct got_error *err = NULL;
 	struct got_fileindex_entry *ie = NULL;
 	struct stat sb;
-	char *ondisk_path, *parent = NULL;
+	char *ondisk_path;
 
 	ie = got_fileindex_entry_get(a->fileindex, relpath, strlen(relpath));
 	if (ie == NULL)
@@ -3866,6 +3901,8 @@ schedule_for_deletion(void *arg, unsigned char status,
 	}
 
 	if (!a->keep_on_disk && status != GOT_STATUS_MISSING) {
+		size_t root_len;
+
 		if (dirfd != -1) {
 			if (unlinkat(dirfd, de_name, 0) != 0) {
 				err = got_error_from_errno2("unlinkat",
@@ -3877,25 +3914,22 @@ schedule_for_deletion(void *arg, unsigned char status,
 			goto done;
 		}
 
-		parent = dirname(ondisk_path);
-
-		if (parent == NULL) {
-			err = got_error_from_errno2("dirname", ondisk_path);
-			goto done;
-		}
-		while (parent && strcmp(parent, a->worktree->root_path) != 0) {
-			if (rmdir(parent) == -1) {
+		root_len = strlen(a->worktree->root_path);
+		do {
+			char *parent;
+			err = got_path_dirname(&parent, ondisk_path);
+			if (err)
+				return err;
+			free(ondisk_path);
+			ondisk_path = parent;
+			if (rmdir(ondisk_path) == -1) {
 				if (errno != ENOTEMPTY)
 					err = got_error_from_errno2("rmdir",
-					    parent);
+					    ondisk_path);
 				break;
 			}
-			parent = dirname(parent);
-			if (parent == NULL) {
-				err = got_error_from_errno2("dirname", parent);
-				goto done;
-			}
-		}
+		} while (got_path_cmp(ondisk_path, a->worktree->root_path,
+		    strlen(ondisk_path), root_len) != 0);
 	}
 
 	got_fileindex_entry_mark_deleted_from_disk(ie);
blob - a99744c546674ba7c12f31a138da72c698c6902d
blob + a0db9b1a7f0df182ae32cc5af74379a2ad21c6d8
--- tog/tog.c
+++ tog/tog.c
@@ -2439,30 +2439,34 @@ input_log_view(struct tog_view **new_view, struct tog_
 			*new_view = tree_view;
 		break;
 	case KEY_BACKSPACE:
-		if (strcmp(s->in_repo_path, "/") == 0)
+		if (got_path_cmp(s->in_repo_path, "/",
+		    strlen(s->in_repo_path), 1) == 0)
 			break;
-		parent_path = dirname(s->in_repo_path);
-		if (parent_path && strcmp(parent_path, ".") != 0) {
-			err = stop_log_thread(s);
-			if (err)
-				return err;
-			lv = view_open(view->nlines, view->ncols,
-			    view->begin_y, view->begin_x, TOG_VIEW_LOG);
-			if (lv == NULL)
-				return got_error_from_errno(
-				    "view_open");
-			err = open_log_view(lv, s->start_id, s->refs,
-			    s->repo, s->head_ref_name, parent_path,
-			    s->log_branches);
-			if (err)
-				return err;;
-			if (view_is_parent_view(view))
-				*new_view = lv;
-			else {
-				view_set_child(view->parent, lv);
-				*focus_view = lv;
-			}
-			return NULL;
+		err = got_path_dirname(&parent_path, s->in_repo_path);
+		if (err)
+			return err;
+		err = stop_log_thread(s);
+		if (err) {
+			free(parent_path);
+			return err;
+		}
+		lv = view_open(view->nlines, view->ncols,
+		    view->begin_y, view->begin_x, TOG_VIEW_LOG);
+		if (lv == NULL) {
+			free(parent_path);
+			return got_error_from_errno("view_open");
+		}
+		err = open_log_view(lv, s->start_id, s->refs,
+		    s->repo, s->head_ref_name, parent_path,
+		    s->log_branches);
+		free(parent_path);
+		if (err)
+			return err;;
+		if (view_is_parent_view(view))
+			*new_view = lv;
+		else {
+			view_set_child(view->parent, lv);
+			*focus_view = lv;
 		}
 		break;
 	case CTRL('l'):