Download raw body.
tog: lock mutex on script error + blame view deallocation tweak
Below are two diffs. Both of which apply on top of the pending blame regress diff, but they _might_ apply without it. First is short and simple: while working on something else, I noticed that we fail to relock the mutex if we return from view_loop() early due to a tog_read_script_key() error. diff /home/mark/src/got commit - 6419890e5180822b712d47c23cc4d30e63966085 path + /home/mark/src/got blob - 232ab4c8aa553e2c4cf0a880fe27470a40b63e0f file + tog/tog.c --- tog/tog.c +++ tog/tog.c @@ -1707,8 +1707,10 @@ view_input(struct tog_view **new, int *done, struct to if (using_mock_io) { err = tog_read_script_key(tog_io.f, &ch, done); - if (err) + if (err) { + errcode = pthread_mutex_lock(&tog_mutex); return err; + } } else if (view->count && --view->count) { cbreak(); nodelay(view->window, TRUE); Next, while more closely studying the recent view leak fix in 2ca2f982e, I realised we need to treat the blame view case a little differently. Specifically, we only want to free the tog_view structure if open_blame_view() fails--not on any error case in cmd_blame() because if we make it to a view_loop() call in cmd_blame(), view gets freed at the end of view_loop() irrespective of error. And apparently simply setting view to NULL inside view_close() wasn't enough for the (view != NULL) guard. So I've moved view_open() to immediately before the open_blame_view() call, and moved the view->close/view_close() calls to immediately after it so they are only called when open_blame_view() returns an error. The above is not a problem in the other views, because we are calling view_close/view->close from the open_*_view() routines--not cmd_*(); that is, view_loop() is never entered in those cases. I've attached a diff(1) patch of this because 'got diff' doesn't show this change as cleanly. --- tog.c Sat Apr 15 22:29:33 2023 +++ tog/tog.c Sat Apr 15 22:25:51 2023 @@ -1707,8 +1707,10 @@ view_input(struct tog_view **new, int *done, struct to if (using_mock_io) { err = tog_read_script_key(tog_io.f, &ch, done); - if (err) + if (err) { + errcode = pthread_mutex_lock(&tog_mutex); return err; + } } else if (view->count && --view->count) { cbreak(); nodelay(view->window, TRUE); @@ -6997,12 +6999,6 @@ cmd_blame(int argc, char *argv[]) if (error != NULL) goto done; - view = view_open(0, 0, 0, 0, TOG_VIEW_BLAME); - if (view == NULL) { - error = got_error_from_errno("view_open"); - goto done; - } - error = got_object_open_as_commit(&commit, repo, commit_id); if (error) goto done; @@ -7012,10 +7008,19 @@ cmd_blame(int argc, char *argv[]) if (error) goto done; + view = view_open(0, 0, 0, 0, TOG_VIEW_BLAME); + if (view == NULL) { + error = got_error_from_errno("view_open"); + goto done; + } error = open_blame_view(view, link_target ? link_target : in_repo_path, commit_id, repo); - if (error) + if (error != NULL) { + if (view->close == NULL) + close_blame_view(view); + view_close(view); goto done; + } if (worktree) { /* Release work tree lock. */ got_worktree_close(worktree); @@ -7028,11 +7033,6 @@ done: free(link_target); free(cwd); free(commit_id); - if (error != NULL && view != NULL) { - if (view->close == NULL) - close_blame_view(view); - view_close(view); - } if (commit) got_object_commit_close(commit); if (worktree) -- Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org> GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
tog: lock mutex on script error + blame view deallocation tweak