Code

Introduce io_prepare as a fix to plug memory leaks related to argv formatting
[tig.git] / tig.c
diff --git a/tig.c b/tig.c
index 49ef43727a3925d409276907fe0241ecb7023261..46f289ec1335debbcff6bc0ba014455bf7d06c77 100644 (file)
--- a/tig.c
+++ b/tig.c
@@ -678,6 +678,24 @@ argv_from_env(const char **argv, const char *name)
        return !env || argv_from_string(argv, &argc, env);
 }
 
+static void
+argv_free(const char *argv[])
+{
+       int argc;
+
+       for (argc = 0; argv[argc]; argc++)
+               free((void *) argv[argc]);
+}
+
+static void
+argv_copy(const char *dst[], const char *src[])
+{
+       int argc;
+
+       for (argc = 0; src[argc]; argc++)
+               dst[argc] = src[argc];
+}
+
 
 /*
  * Executing external commands.
@@ -725,6 +743,13 @@ io_init(struct io *io, const char *dir, enum io_type type)
        io->dir = dir;
 }
 
+static void
+io_prepare(struct io *io, const char *dir, enum io_type type, const char *argv[])
+{
+       io_init(io, dir, type);
+       argv_copy(io->argv, argv);
+}
+
 static bool
 io_format(struct io *io, const char *dir, enum io_type type,
          const char *argv[], enum format_flags flags)
@@ -850,9 +875,7 @@ io_start(struct io *io)
 static bool
 io_run(struct io *io, const char **argv, const char *dir, enum io_type type)
 {
-       io_init(io, dir, type);
-       if (!format_argv(io->argv, argv, FORMAT_NONE))
-               return FALSE;
+       io_prepare(io, dir, type, argv);
        return io_start(io);
 }
 
@@ -867,8 +890,7 @@ io_run_bg(const char **argv)
 {
        struct io io = {};
 
-       if (!io_format(&io, NULL, IO_BG, argv, FORMAT_NONE))
-               return FALSE;
+       io_prepare(&io, NULL, IO_BG, argv);
        return io_complete(&io);
 }
 
@@ -877,29 +899,24 @@ io_run_fg(const char **argv, const char *dir)
 {
        struct io io = {};
 
-       if (!io_format(&io, dir, IO_FG, argv, FORMAT_NONE))
-               return FALSE;
+       io_prepare(&io, dir, IO_FG, argv);
        return io_complete(&io);
 }
 
 static bool
-io_run_append(const char **argv, enum format_flags flags, int fd)
+io_run_append(const char **argv, int fd)
 {
        struct io io = {};
 
-       if (!io_format(&io, NULL, IO_AP, argv, flags)) {
-               close(fd);
-               return FALSE;
-       }
-
+       io_prepare(&io, NULL, IO_AP, argv);
        io.pipe = fd;
        return io_complete(&io);
 }
 
 static bool
-io_run_rd(struct io *io, const char **argv, const char *dir, enum format_flags flags)
+io_run_rd(struct io *io, const char **argv, const char *dir)
 {
-       return io_format(io, dir, IO_RD, argv, flags) && io_start(io);
+       return io_format(io, dir, IO_RD, argv, FORMAT_NONE) && io_start(io);
 }
 
 static bool
@@ -1036,8 +1053,8 @@ io_run_buf(const char **argv, char buf[], size_t bufsize)
 {
        struct io io = {};
 
-       return io_run_rd(&io, argv, NULL, FORMAT_NONE)
-           && io_read_buf(&io, buf, bufsize);
+       io_prepare(&io, NULL, IO_RD, argv);
+       return io_start(&io) && io_read_buf(&io, buf, bufsize);
 }
 
 static int
@@ -1084,8 +1101,8 @@ io_run_load(const char **argv, const char *separators,
 {
        struct io io = {};
 
-       return io_format(&io, NULL, IO_RD, argv, FORMAT_NONE)
-               ? io_load(&io, separators, read_property) : ERR;
+       io_prepare(&io, NULL, IO_RD, argv);
+       return io_load(&io, separators, read_property);
 }
 
 
@@ -1279,6 +1296,8 @@ LINE(AUTHOR,         "author ",           COLOR_GREEN,    COLOR_DEFAULT,  0), \
 LINE(COMMITTER,           "committer ",        COLOR_MAGENTA,  COLOR_DEFAULT,  0), \
 LINE(SIGNOFF,     "    Signed-off-by", COLOR_YELLOW,   COLOR_DEFAULT,  0), \
 LINE(ACKED,       "    Acked-by",      COLOR_YELLOW,   COLOR_DEFAULT,  0), \
+LINE(TESTED,      "    Tested-by",     COLOR_YELLOW,   COLOR_DEFAULT,  0), \
+LINE(REVIEWED,    "    Reviewed-by",   COLOR_YELLOW,   COLOR_DEFAULT,  0), \
 LINE(DEFAULT,     "",                  COLOR_DEFAULT,  COLOR_DEFAULT,  A_NORMAL), \
 LINE(CURSOR,      "",                  COLOR_WHITE,    COLOR_GREEN,    A_BOLD), \
 LINE(STATUS,      "",                  COLOR_GREEN,    COLOR_DEFAULT,  0), \
@@ -1522,6 +1541,14 @@ static void
 add_keybinding(enum keymap keymap, enum request request, int key)
 {
        struct keybinding_table *table = &keybindings[keymap];
+       size_t i;
+
+       for (i = 0; i < keybindings[keymap].size; i++) {
+               if (keybindings[keymap].data[i].alias == key) {
+                       keybindings[keymap].data[i].request = request;
+                       return;
+               }
+       }
 
        table->data = realloc(table->data, (table->size + 1) * sizeof(*table->data));
        if (!table->data)
@@ -1762,8 +1789,10 @@ add_builtin_run_requests(void)
        int i;
 
        for (i = 0; i < ARRAY_SIZE(reqs); i++) {
-               enum request req;
+               enum request req = get_keybinding(reqs[i].keymap, reqs[i].key);
 
+               if (req != reqs[i].key)
+                       continue;
                req = add_run_request(reqs[i].keymap, reqs[i].key, reqs[i].argc, reqs[i].argv);
                if (req != REQ_NONE)
                        add_keybinding(reqs[i].keymap, req, reqs[i].key);
@@ -2137,8 +2166,6 @@ load_options(void)
        const char *tigrc_system = getenv("TIGRC_SYSTEM");
        char buf[SIZEOF_STR];
 
-       add_builtin_run_requests();
-
        if (!tigrc_system)
                tigrc_system = SYSCONFDIR "/tigrc";
        load_option_file(tigrc_system);
@@ -2150,6 +2177,10 @@ load_options(void)
        }
        load_option_file(tigrc_user);
 
+       /* Add _after_ loading config files to avoid adding run requests
+        * that conflict with keybindings. */
+       add_builtin_run_requests();
+
        return OK;
 }
 
@@ -2200,7 +2231,6 @@ struct view {
 
        enum keymap keymap;     /* What keymap does this view have */
        bool git_dir;           /* Whether the view requires a git directory. */
-       bool refresh;           /* Whether the view supports refreshing. */
 
        char ref[SIZEOF_REF];   /* Hovered commit reference */
        char vid[SIZEOF_REF];   /* View ID. Set to id member when updating. */
@@ -2225,6 +2255,7 @@ struct view {
        /* If non-NULL, points to the view that opened this view. If this view
         * is closed tig will switch back to the parent view. */
        struct view *parent;
+       struct view *prev;
 
        /* Buffering */
        size_t lines;           /* Total number of lines */
@@ -2277,24 +2308,24 @@ static struct view_ops status_ops;
 static struct view_ops tree_ops;
 static struct view_ops branch_ops;
 
-#define VIEW_STR(type, name, env, ref, ops, map, git, refresh) \
-       { type, name, #env, ref, ops, map, git, refresh }
+#define VIEW_STR(type, name, env, ref, ops, map, git) \
+       { type, name, #env, ref, ops, map, git }
 
-#define VIEW_(id, name, ops, git, refresh, ref) \
-       VIEW_STR(VIEW_##id, name, TIG_##id##_CMD, ref, ops, KEYMAP_##id, git, refresh)
+#define VIEW_(id, name, ops, git, ref) \
+       VIEW_STR(VIEW_##id, name, TIG_##id##_CMD, ref, ops, KEYMAP_##id, git)
 
 static struct view views[] = {
-       VIEW_(MAIN,   "main",   &main_ops,   TRUE,  TRUE,  ref_head),
-       VIEW_(DIFF,   "diff",   &diff_ops,   TRUE,  FALSE, ref_commit),
-       VIEW_(LOG,    "log",    &log_ops,    TRUE,  TRUE,  ref_head),
-       VIEW_(TREE,   "tree",   &tree_ops,   TRUE,  FALSE, ref_commit),
-       VIEW_(BLOB,   "blob",   &blob_ops,   TRUE,  FALSE, ref_blob),
-       VIEW_(BLAME,  "blame",  &blame_ops,  TRUE,  FALSE, ref_commit),
-       VIEW_(BRANCH, "branch", &branch_ops, TRUE,  TRUE,  ref_head),
-       VIEW_(HELP,   "help",   &help_ops,   FALSE, FALSE, ""),
-       VIEW_(PAGER,  "pager",  &pager_ops,  FALSE, FALSE, "stdin"),
-       VIEW_(STATUS, "status", &status_ops, TRUE,  TRUE,  ""),
-       VIEW_(STAGE,  "stage",  &stage_ops,  TRUE,  TRUE,  ""),
+       VIEW_(MAIN,   "main",   &main_ops,   TRUE,  ref_head),
+       VIEW_(DIFF,   "diff",   &diff_ops,   TRUE,  ref_commit),
+       VIEW_(LOG,    "log",    &log_ops,    TRUE,  ref_head),
+       VIEW_(TREE,   "tree",   &tree_ops,   TRUE,  ref_commit),
+       VIEW_(BLOB,   "blob",   &blob_ops,   TRUE,  ref_blob),
+       VIEW_(BLAME,  "blame",  &blame_ops,  TRUE,  ref_commit),
+       VIEW_(BRANCH, "branch", &branch_ops, TRUE,  ref_head),
+       VIEW_(HELP,   "help",   &help_ops,   FALSE, ""),
+       VIEW_(PAGER,  "pager",  &pager_ops,  FALSE, "stdin"),
+       VIEW_(STATUS, "status", &status_ops, TRUE,  ""),
+       VIEW_(STAGE,  "stage",  &stage_ops,  TRUE,  ""),
 };
 
 #define VIEW(req)      (&views[(req) - REQ_OFFSET - 1])
@@ -2305,8 +2336,18 @@ static struct view views[] = {
 #define view_is_displayed(view) \
        (view == display[0] || view == display[1])
 
-#define view_has_parent(view, child_type, parent_type) \
-       (view->type == child_type && view->parent && view->parent->type == parent_type)
+static enum request
+view_request(struct view *view, enum request request)
+{
+       if (!view || !view->lines)
+               return request;
+       return view->ops->request(view, request, &view->line[view->lineno]);
+}
+
+
+/*
+ * View drawing.
+ */
 
 static inline void
 set_view_attr(struct view *view, enum line_type type)
@@ -2697,6 +2738,11 @@ redraw_display(bool clear)
        }
 }
 
+
+/*
+ * Option management
+ */
+
 static void
 toggle_enum_option_do(unsigned int *opt, const char *help,
                      const struct enum_map *map, size_t size)
@@ -3120,15 +3166,6 @@ reset_view(struct view *view)
        view->update_secs = 0;
 }
 
-static void
-free_argv(const char *argv[])
-{
-       int argc;
-
-       for (argc = 0; argv[argc]; argc++)
-               free((void *) argv[argc]);
-}
-
 static const char *
 format_arg(const char *name)
 {
@@ -3165,7 +3202,7 @@ format_argv(const char *dst_argv[], const char *src_argv[], enum format_flags fl
        int argc;
        bool noreplace = flags == FORMAT_NONE;
 
-       free_argv(dst_argv);
+       argv_free(dst_argv);
 
        for (argc = 0; src_argv[argc]; argc++) {
                const char *arg = src_argv[argc];
@@ -3364,7 +3401,8 @@ update_view(struct view *view)
                end_update(view, TRUE);
 
        } else if (io_eof(view->pipe)) {
-               report("");
+               if (view_is_displayed(view))
+                       report("");
                end_update(view, FALSE);
        }
 
@@ -3461,6 +3499,7 @@ open_view(struct view *prev, enum request request, enum open_flags flags)
        if (split) {
                display[1] = view;
                current_view = 1;
+               view->parent = prev;
        } else if (!nomaximize) {
                /* Maximize the current view. */
                memset(display, 0, sizeof(display));
@@ -3468,9 +3507,9 @@ open_view(struct view *prev, enum request request, enum open_flags flags)
                display[current_view] = view;
        }
 
-       /* No parent signals that this is the first loaded view. */
+       /* No prev signals that this is the first loaded view. */
        if (prev && view != prev) {
-               view->parent = prev;
+               view->prev = prev;
        }
 
        /* Resize the view when switching between split- and full-screen,
@@ -3573,7 +3612,7 @@ open_run_request(enum request request)
 
        if (format_argv(argv, req->argv, FORMAT_ALL))
                open_external_viewer(argv, NULL);
-       free_argv(argv);
+       argv_free(argv);
 }
 
 /*
@@ -3590,18 +3629,13 @@ view_driver(struct view *view, enum request request)
 
        if (request > REQ_NONE) {
                open_run_request(request);
-               /* FIXME: When all views can refresh always do this. */
-               if (view->refresh)
-                       request = REQ_REFRESH;
-               else
-                       return TRUE;
+               view_request(view, REQ_REFRESH);
+               return TRUE;
        }
 
-       if (view && view->lines) {
-               request = view->ops->request(view, request, &view->line[view->lineno]);
-               if (request == REQ_NONE)
-                       return TRUE;
-       }
+       request = view_request(view, request);
+       if (request == REQ_NONE)
+               return TRUE;
 
        switch (request) {
        case REQ_MOVE_UP:
@@ -3679,11 +3713,7 @@ view_driver(struct view *view, enum request request)
        case REQ_PREVIOUS:
                request = request == REQ_NEXT ? REQ_MOVE_DOWN : REQ_MOVE_UP;
 
-               if (view_has_parent(view, VIEW_DIFF, VIEW_MAIN) ||
-                   view_has_parent(view, VIEW_DIFF, VIEW_BLAME) ||
-                   view_has_parent(view, VIEW_STAGE, VIEW_STATUS) ||
-                   view_has_parent(view, VIEW_BLOB, VIEW_TREE) ||
-                   view_has_parent(view, VIEW_MAIN, VIEW_BRANCH)) {
+               if (view->parent) {
                        int line;
 
                        view = view->parent;
@@ -3692,9 +3722,7 @@ view_driver(struct view *view, enum request request)
                        if (view_is_displayed(view))
                                update_view_title(view);
                        if (line != view->lineno)
-                               view->ops->request(view, REQ_ENTER,
-                                                  &view->line[view->lineno]);
-
+                               view_request(view, REQ_ENTER);
                } else {
                        move_view(view, request);
                }
@@ -3789,13 +3817,12 @@ view_driver(struct view *view, enum request request)
                break;
 
        case REQ_VIEW_CLOSE:
-               /* XXX: Mark closed views by letting view->parent point to the
+               /* XXX: Mark closed views by letting view->prev point to the
                 * view itself. Parents to closed view should never be
                 * followed. */
-               if (view->parent &&
-                   view->parent->parent != view->parent) {
-                       maximize_view(view->parent);
-                       view->parent = view;
+               if (view->prev && view->prev != view) {
+                       maximize_view(view->prev);
+                       view->prev = view;
                        break;
                }
                /* Fall-through */
@@ -4004,7 +4031,9 @@ select_commit_parent(const char *id, char rev[SIZEOF_REV], const char *path)
                return FALSE;
        }
 
-       if (parents > 1 && !open_commit_parent_menu(buf, &parents))
+       if (parents == 1)
+               parents = 0;
+       else if (!open_commit_parent_menu(buf, &parents))
                return FALSE;
 
        string_copy_rev(rev, &buf[41 * parents]);
@@ -4506,7 +4535,7 @@ tree_read_date(struct view *view, char *text, bool *read_date)
                        return TRUE;
                }
 
-               if (!io_run_rd(&io, log_file, opt_cdup, FORMAT_NONE)) {
+               if (!io_run_rd(&io, log_file, opt_cdup)) {
                        report("Failed to load tree data");
                        return TRUE;
                }
@@ -4641,14 +4670,15 @@ tree_draw(struct view *view, struct line *line, unsigned int lineno)
 }
 
 static void
-open_blob_editor()
+open_blob_editor(const char *id)
 {
+       const char *blob_argv[] = { "git", "cat-file", "blob", id, NULL };
        char file[SIZEOF_STR] = "/tmp/tigblob.XXXXXX";
        int fd = mkstemp(file);
 
        if (fd == -1)
                report("Failed to create temporary file");
-       else if (!io_run_append(blob_ops.argv, FORMAT_ALL, fd))
+       else if (!io_run_append(blob_argv, fd))
                report("Failed to save blob data to file");
        else
                open_editor(file);
@@ -4660,6 +4690,7 @@ static enum request
 tree_request(struct view *view, enum request request, struct line *line)
 {
        enum open_flags flags;
+       struct tree_entry *entry = line->data;
 
        switch (request) {
        case REQ_VIEW_BLAME:
@@ -4675,7 +4706,7 @@ tree_request(struct view *view, enum request request, struct line *line)
                if (line->type != LINE_TREE_FILE) {
                        report("Edit only supported for files");
                } else if (!is_head_commit(view->vid)) {
-                       open_blob_editor();
+                       open_blob_editor(entry->id);
                } else {
                        open_editor(opt_file);
                }
@@ -4726,7 +4757,7 @@ tree_request(struct view *view, enum request request, struct line *line)
                break;
 
        case LINE_TREE_FILE:
-               flags = display[0] == view ? OPEN_SPLIT : OPEN_DEFAULT;
+               flags = view_is_displayed(view) ? OPEN_SPLIT : OPEN_DEFAULT;
                request = REQ_VIEW_BLOB;
                break;
 
@@ -4826,7 +4857,7 @@ blob_request(struct view *view, enum request request, struct line *line)
 {
        switch (request) {
        case REQ_EDIT:
-               open_blob_editor();
+               open_blob_editor(view->vid);
                return REQ_NONE;
        default:
                return pager_request(view, request, line);
@@ -4859,18 +4890,6 @@ static struct view_ops blob_ops = {
  *     reading output from git-blame.
  */
 
-static const char *blame_head_argv[] = {
-       "git", "blame", "--incremental", "--", "%(file)", NULL
-};
-
-static const char *blame_ref_argv[] = {
-       "git", "blame", "--incremental", "%(ref)", "--", "%(file)", NULL
-};
-
-static const char *blame_cat_file_argv[] = {
-       "git", "cat-file", "blob", "%(ref):%(file)", NULL
-};
-
 struct blame_commit {
        char id[SIZEOF_REV];            /* SHA1 ID. */
        char title[128];                /* First line of the commit message. */
@@ -4891,14 +4910,19 @@ blame_open(struct view *view)
 {
        char path[SIZEOF_STR];
 
-       if (!view->parent && *opt_prefix) {
+       if (!view->prev && *opt_prefix) {
                string_copy(path, opt_file);
                if (!string_format(opt_file, "%s%s", opt_prefix, path))
                        return FALSE;
        }
 
        if (*opt_ref || !io_open(&view->io, "%s%s", opt_cdup, opt_file)) {
-               if (!io_run_rd(&view->io, blame_cat_file_argv, opt_cdup, FORMAT_ALL))
+               const char *blame_cat_file_argv[] = {
+                       "git", "cat-file", "blob", path, NULL
+               };
+
+               if (!string_format(path, "%s:%s", opt_ref, opt_file) ||
+                   !io_run_rd(&view->io, blame_cat_file_argv, opt_cdup))
                        return FALSE;
        }
 
@@ -4988,13 +5012,16 @@ static bool
 blame_read_file(struct view *view, const char *line, bool *read_file)
 {
        if (!line) {
-               const char **argv = *opt_ref ? blame_ref_argv : blame_head_argv;
+               const char *blame_argv[] = {
+                       "git", "blame", "--incremental",
+                               *opt_ref ? opt_ref : "--incremental", "--", opt_file, NULL
+               };
                struct io io = {};
 
-               if (view->lines == 0 && !view->parent)
+               if (view->lines == 0 && !view->prev)
                        die("No blame exist for %s", view->vid);
 
-               if (view->lines == 0 || !io_run_rd(&io, argv, opt_cdup, FORMAT_ALL)) {
+               if (view->lines == 0 || !io_run_rd(&io, blame_argv, opt_cdup)) {
                        report("Failed to load blame data");
                        return TRUE;
                }
@@ -5163,7 +5190,7 @@ setup_blame_parent_line(struct view *view, struct blame *blame)
 static enum request
 blame_request(struct view *view, enum request request, struct line *line)
 {
-       enum open_flags flags = display[0] == view ? OPEN_SPLIT : OPEN_DEFAULT;
+       enum open_flags flags = view_is_displayed(view) ? OPEN_SPLIT : OPEN_DEFAULT;
        struct blame *blame = line->data;
 
        switch (request) {
@@ -5428,7 +5455,7 @@ branch_open(struct view *view)
                        "--simplify-by-decoration", "--all", NULL
        };
 
-       if (!io_run_rd(&view->io, branch_log, NULL, FORMAT_NONE)) {
+       if (!io_run_rd(&view->io, branch_log, NULL)) {
                report("Failed to load branch data");
                return TRUE;
        }
@@ -5889,7 +5916,7 @@ status_enter(struct view *view, struct line *line)
                die("line type %d not handled in switch", line->type);
        }
 
-       split = view_is_displayed(view) ? OPEN_SPLIT : 0;
+       split = view_is_displayed(view) ? OPEN_SPLIT : OPEN_DEFAULT;
        open_view(view, REQ_VIEW_STAGE, OPEN_PREPARED | split);
        if (view_is_displayed(VIEW(REQ_VIEW_STAGE))) {
                if (status) {
@@ -6769,7 +6796,7 @@ main_read(struct view *view, char *line)
        if (!line) {
                int i;
 
-               if (!view->lines && !view->parent)
+               if (!view->lines && !view->prev)
                        die("No revisions match the given arguments.");
                if (view->lines > 0) {
                        commit = view->line[view->lines - 1].data;
@@ -6859,7 +6886,7 @@ main_read(struct view *view, char *line)
 static enum request
 main_request(struct view *view, enum request request, struct line *line)
 {
-       enum open_flags flags = display[0] == view ? OPEN_SPLIT : OPEN_DEFAULT;
+       enum open_flags flags = view_is_displayed(view) ? OPEN_SPLIT : OPEN_DEFAULT;
 
        switch (request) {
        case REQ_ENTER: