Mark Jamsek <mark@jamsek.com> wrote:
> Mark Jamsek <mark@jamsek.com> wrote:
> > As per the subject line, I introduced a NULL pointer dereference in
> > a66bb11379 "implement tog work tree diff support via log view and CLI"
> > when the T keymap is used on one of the new work tree entries. This is
> > because the commit_queue_entry id member is a NULL pointer for work tree
> > entries, and it is unconditionally dereferenced when passed to
> > open_tree_view().
> > 
> > Instead, use the work tree's base commit, which we must have if a work
> > tree entry is selected, to display the tree view.
> > 
> > 
> > commit 562100e0c5c3e9a191f9e7dc903d8c36d4ec9993 (main)
> > from: Mark Jamsek <mark@jamsek.dev>
> > date: Sun Dec 29 16:14:22 2024 UTC
> > 
> > fix segv when using T keymap on log view worktree entry
> > 
> > When using the T keymap in the log view, we pass the selected_entry
> > to browse_commit_tree() to open a tree view of the repository as at
> > the selected commit, which passes the selected_entry->id member to
> > open_tree_view() to open the commit. The id member, which is a NULL
> > pointer in work tree entries, is dereferenced and we segfault.
> > If the T keymap is used on a work tree entry, use the work tree base
> > commit id instead, which we must have if a work tree entry is selected.
> 
> Please disregard the previous diff, it leaked tree_view on error.
> 
> Updated diff below.

And here's the actual updated diff; sorry for the noise!


commit 719815ce9594454cb83f7e945f076d4d696b6eae (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Sun Dec 29 16:46:47 2024 UTC

fix segv when using T keymap on log view worktree entry

When using the T keymap in the log view, we pass the selected_entry
to browse_commit_tree() to open a tree view of the repository as at
the selected commit, which passes the selected_entry->id member to
open_tree_view() to open the commit. The id member, which is a NULL
pointer in work tree entries, is dereferenced and we segfault.
If the T keymap is used on a work tree entry, use the base commit id
instead, which we must have if a work tree entry is selected.

M  tog/tog.c  |  31+  6-

1 file changed, 31 insertions(+), 6 deletions(-)

commit - 8e5958b86dfbd58e1637258b72b19c17ce5c8503
commit + 719815ce9594454cb83f7e945f076d4d696b6eae
blob - 368c057ac8f74f6cc9f839c25c94eaa812c22b53
blob + 5ec86d7055e217c0f6fc07335ccbaea2c8ff4b42
--- tog/tog.c
+++ tog/tog.c
@@ -3841,22 +3841,47 @@ browse_commit_tree(struct tog_view **new_view, int beg
 	const struct got_error *err = NULL;
 	struct tog_tree_view_state *s;
 	struct tog_view *tree_view;
+	struct got_commit_object *commit = NULL;
+	struct got_object_id *commit_id;
 
 	tree_view = view_open(0, 0, begin_y, begin_x, TOG_VIEW_TREE);
 	if (tree_view == NULL)
 		return got_error_from_errno("view_open");
 
-	err = open_tree_view(tree_view, entry->id, head_ref_name, repo);
+	if (entry->id != NULL)
+		commit_id = entry->id;
+	else if (entry->worktree_entry)
+		commit_id = tog_base_commit.id;
+	else {
+		/* cannot happen */
+		err = got_error(GOT_ERR_NOT_WORKTREE);
+		goto done;
+	}
+
+	err = open_tree_view(tree_view, commit_id, head_ref_name, repo);
 	if (err)
-		return err;
+		goto done;
 	s = &tree_view->state.tree;
 
-	*new_view = tree_view;
-
 	if (got_path_is_root_dir(path))
-		return NULL;
+		goto done;
 
-	return tree_view_walk_path(s, entry->commit, path);
+	if (entry->worktree_entry) {
+		err = got_object_open_as_commit(&commit, repo, commit_id);
+		if (err != NULL)
+			goto done;
+	}
+
+	err = tree_view_walk_path(s, commit ? commit : entry->commit, path);
+
+done:
+	if (commit != NULL)
+		got_object_commit_close(commit);
+	if (err == NULL)
+		*new_view = tree_view;
+	else
+		free(tree_view);
+	return err;
 }
 
 /*


-- 
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68