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!

> diff /home/op/w/gotacl
> commit - 276bccc4651afa03134d0faa1cf1c703b5f819bd
> path + /home/op/w/gotacl
> blob - 25bddbc9c0a8c95dc7b5008c82de519be7d36960
> file + tog/tog.c
> --- tog/tog.c
> +++ tog/tog.c
> @@ -2401,6 +2401,7 @@ draw_commit(struct tog_view *view, struct got_commit_o
>  	struct tog_log_view_state *s = &view->state.log;
>  	const struct got_error *err = NULL;
>  	char datebuf[12]; /* YYYY-MM-DD + SPACE + NUL */
> +	char *refs_str = NULL;
>  	char *logmsg0 = NULL, *logmsg = NULL;
>  	char *author = NULL;
>  	wchar_t *wlogmsg = NULL, *wauthor = NULL;
> @@ -2495,13 +2496,13 @@ 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) {
> -		char *refs_str, *newlogmsg;
> -		wchar_t *ws;
> -
> +	if (refs)
>  		err = build_refs_str(&refs_str, refs, id, s->repo);
> -		if (err)
> -			goto done;
> +	if (err)
> +		goto done;
> +	if (refs_str) {
> +		char *newlogmsg;
> +		wchar_t *ws;
>  	
>  		/*
>  		 * The length of this wide-char sub-string will be
> @@ -2516,10 +2517,8 @@ draw_commit(struct tog_view *view, struct got_commit_o
>  
>  		if (asprintf(&newlogmsg, "[%s] %s", refs_str, logmsg) == -1) {
>  			err = got_error_from_errno("asprintf");
> -			free(refs_str);
>  			goto done;
>  		}
> -		free(refs_str);
>  
>  		free(logmsg0);
>  		logmsg0 = newlogmsg;
> @@ -2554,6 +2553,7 @@ done:
>  done:
>  	free(logmsg0);
>  	free(wlogmsg);
> +	free(refs_str);
>  	free(author);
>  	free(wauthor);
>  	free(line);
>