On 2023/05/16 23:46:26 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Tue, May 16, 2023 at 11:30:45AM +0200, Omar Polo wrote:
> > I've found that my suggestion could result in a segfault.  This
> > because the reflist may include uninteresting refs in the refs/got/
> > namespace that gets filtered out by build_refs_str.
> > 
> > If a commit happens to have only uninteresting refs pointing to it
> > (e.g. when you have worktrees not up-to-date), build_refs_str returns
> > NULL, whihc is not handled here.
> > 
> > I'm attaching a diff with a way to handle it.  Another way would be to
> > relax build_refs_str to allow a NULL refs being passed.
> 
> ok, thanks!

as a small follow-up, what about allowing to pass a NULL reflist to
build_refs_str?  Simplifies a bit the callers.

diffstat /home/op/w/gotacl
 M  tog/tog.c  |  11+  13-

1 file changed, 11 insertions(+), 13 deletions(-)

diff /home/op/w/gotacl
commit - 29efeeddc18828c36f1a6df50e05297a51a1a396
path + /home/op/w/gotacl
blob - 6d3e738e1220743df1fd30ba6a2779ff5b504e3e
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -2322,6 +2322,9 @@ build_refs_str(char **refs_str, struct got_reflist_hea
 
 	*refs_str = NULL;
 
+	if (refs == NULL)
+		return NULL;
+
 	TAILQ_FOREACH(re, refs, entry) {
 		struct got_tag_object *tag = NULL;
 		struct got_object_id *ref_id;
@@ -2496,8 +2499,7 @@ draw_commit(struct tog_view *view, struct got_commit_o
 
 	/* Prepend reference labels to log message if possible .*/
 	refs = got_reflist_object_id_map_lookup(tog_refs_idmap, id);
-	if (refs)
-		err = build_refs_str(&refs_str, refs, id, s->repo);
+	err = build_refs_str(&refs_str, refs, id, s->repo);
 	if (err)
 		goto done;
 	if (refs_str) {
@@ -2777,12 +2779,10 @@ draw_commits(struct tog_view *view)
 			return err;
 		refs = got_reflist_object_id_map_lookup(tog_refs_idmap,
 		    s->selected_entry->id);
-		if (refs) {
-			err = build_refs_str(&refs_str, refs,
-			    s->selected_entry->id, s->repo);
-			if (err)
-				goto done;
-		}
+		err = build_refs_str(&refs_str, refs, s->selected_entry->id,
+		    s->repo);
+		if (err)
+			goto done;
 	}
 
 	if (s->thread_args.commits_needed == 0 && !using_mock_io)
@@ -4965,11 +4965,9 @@ write_commit_info(struct got_diff_line **lines, size_t
 	off_t outoff = 0;
 	int n;
 
-	if (refs) {
-		err = build_refs_str(&refs_str, refs, commit_id, repo);
-		if (err)
-			return err;
-	}
+	err = build_refs_str(&refs_str, refs, commit_id, repo);
+	if (err)
+		return err;
 
 	err = got_object_open_as_commit(&commit, repo, commit_id);
 	if (err)