Commit 213556435a701b3010c66843b5b824af2d3b6f72
2020-12-06T22:03:32
fix crashes when the 'tog log' view reloads displayed data This reimplements log view reloading (Ctrl-L), logging of a parent path (Backspace), and the toggle to show commits on branches (B). The idea is to reuse the existing log view and change its state, instead of allocating a new view with a new state and replacing the existing view. Fixes a segfault that occurs when a parent path is logged with Backspace: tog tree -r got.git -c 0.44 pick tog/tog.c 'l' Backspace -> tog will segfault The first change in this patch is a partial fix. The log thread should always check the 'quit' flag as soon as it wakes from sleep. Otherwise it could try to load more commits after waking up and before checking the 'quit' flag. It will then attempt to load commits with a NULL commit graph pointer. This partial fix by itself is not sufficient to fix the crash, since we'll now see a bus error in the main thread, instead of a NULL deref in the log thread. The remainder of the patch fixes this bus error. ok naddy
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151
diff --git a/tog/tog.c b/tog/tog.c
index 7a0e74a..9e53eff 100644
--- a/tog/tog.c
+++ b/tog/tog.c
@@ -2026,6 +2026,8 @@ log_thread(void *arg)
if (errcode)
err = got_error_set_errno(errcode,
"pthread_cond_wait");
+ if (*a->quit)
+ done = 1;
}
}
@@ -2348,11 +2350,9 @@ input_log_view(struct tog_view **new_view, struct tog_view *view, int ch)
{
const struct got_error *err = NULL;
struct tog_log_view_state *s = &view->state.log;
- char *parent_path, *in_repo_path = NULL;
- struct tog_view *diff_view = NULL, *tree_view = NULL, *lv = NULL;
+ struct tog_view *diff_view = NULL, *tree_view = NULL;
struct tog_view *ref_view = NULL;
int begin_x = 0;
- struct got_object_id *start_id;
switch (ch) {
case 'q':
@@ -2473,83 +2473,55 @@ input_log_view(struct tog_view **new_view, struct tog_view *view, int ch)
*new_view = tree_view;
break;
case KEY_BACKSPACE:
- if (got_path_cmp(s->in_repo_path, "/",
- strlen(s->in_repo_path), 1) == 0)
+ case CTRL('l'):
+ case 'B':
+ if (ch == KEY_BACKSPACE &&
+ got_path_is_root_dir(s->in_repo_path))
break;
- 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->repo, s->head_ref_name,
- parent_path, s->log_branches);
- free(parent_path);
if (err)
- return err;;
- view->focussed = 0;
- lv->focussed = 1;
- if (view_is_parent_view(view))
- *new_view = lv;
- else
- view_set_child(view->parent, lv);
- break;
- case CTRL('l'):
- err = stop_log_thread(s);
+ return err;
+ if (ch == KEY_BACKSPACE) {
+ char *parent_path;
+ err = got_path_dirname(&parent_path, s->in_repo_path);
+ if (err)
+ return err;
+ free(s->in_repo_path);
+ s->in_repo_path = parent_path;
+ s->thread_args.in_repo_path = s->in_repo_path;
+ } else if (ch == CTRL('l')) {
+ struct got_object_id *start_id;
+ err = got_repo_match_object_id(&start_id, NULL,
+ s->head_ref_name ? s->head_ref_name : GOT_REF_HEAD,
+ GOT_OBJ_TYPE_COMMIT, 1, s->repo);
+ if (err)
+ return err;
+ free(s->start_id);
+ s->start_id = start_id;
+ s->thread_args.start_id = s->start_id;
+ } else /* 'B' */
+ s->log_branches = !s->log_branches;
+
+ err = got_repo_open(&s->thread_args.repo,
+ got_repo_get_path(s->repo), NULL);
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 = got_repo_match_object_id(&start_id, NULL,
- s->head_ref_name ? s->head_ref_name : GOT_REF_HEAD,
- GOT_OBJ_TYPE_COMMIT, 1, s->repo);
- if (err) {
- view_close(lv);
+ err = got_commit_graph_open(&s->thread_args.graph,
+ s->in_repo_path, !s->log_branches);
+ if (err)
return err;
- }
- in_repo_path = strdup(s->in_repo_path);
- if (in_repo_path == NULL) {
- free(start_id);
- view_close(lv);
- return got_error_from_errno("strdup");
- }
- err = open_log_view(lv, start_id, s->repo, s->head_ref_name,
- in_repo_path, s->log_branches);
- if (err) {
- free(start_id);
- view_close(lv);
- return err;;
- }
- view->dying = 1;
- *new_view = lv;
- break;
- case 'B':
- s->log_branches = !s->log_branches;
- err = stop_log_thread(s);
+ err = got_commit_graph_iter_start(s->thread_args.graph,
+ s->start_id, s->repo, NULL, NULL);
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->repo,
- s->head_ref_name, s->in_repo_path, s->log_branches);
- if (err) {
- view_close(lv);
- return err;;
- }
- view->dying = 1;
- *new_view = lv;
+ free_commits(&s->commits);
+ s->first_displayed_entry = NULL;
+ s->last_displayed_entry = NULL;
+ s->selected_entry = NULL;
+ s->selected = 0;
+ s->thread_args.log_complete = 0;
+ s->quit = 0;
+ s->thread_args.commits_needed = view->nlines;
break;
case 'r':
if (view_is_parent_view(view))