Skip to content

Commit

Permalink
sequencer: refactor how original todo list lines are accessed
Browse files Browse the repository at this point in the history
Previously, we did a lot of arithmetic gymnastics to get at the line in
the todo list (as stored in todo_list.buf). This might have been fast,
but only in terms of execution speed, not in terms of developer time.

Let's refactor this to make it a lot easier to read, and hence to
reason about the correctness of the code. It is not performance-critical
code anyway.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
dscho authored and gitster committed Apr 26, 2018
1 parent 2f6b1d1 commit a01c2a5
Showing 1 changed file with 36 additions and 24 deletions.
60 changes: 36 additions & 24 deletions sequencer.c
Expand Up @@ -1871,6 +1871,23 @@ static int count_commands(struct todo_list *todo_list)
return count;
}

static int get_item_line_offset(struct todo_list *todo_list, int index)
{
return index < todo_list->nr ?
todo_list->items[index].offset_in_buf : todo_list->buf.len;
}

static const char *get_item_line(struct todo_list *todo_list, int index)
{
return todo_list->buf.buf + get_item_line_offset(todo_list, index);
}

static int get_item_line_length(struct todo_list *todo_list, int index)
{
return get_item_line_offset(todo_list, index + 1)
- get_item_line_offset(todo_list, index);
}

static ssize_t strbuf_read_file_or_whine(struct strbuf *sb, const char *path)
{
int fd;
Expand Down Expand Up @@ -2250,29 +2267,27 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
if (fd < 0)
return error_errno(_("could not lock '%s'"), todo_path);
offset = next < todo_list->nr ?
todo_list->items[next].offset_in_buf : todo_list->buf.len;
offset = get_item_line_offset(todo_list, next);
if (write_in_full(fd, todo_list->buf.buf + offset,
todo_list->buf.len - offset) < 0)
return error_errno(_("could not write to '%s'"), todo_path);
if (commit_lock_file(&todo_lock) < 0)
return error(_("failed to finalize '%s'"), todo_path);

if (is_rebase_i(opts)) {
const char *done_path = rebase_path_done();
int fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
int prev_offset = !next ? 0 :
todo_list->items[next - 1].offset_in_buf;
if (is_rebase_i(opts) && next > 0) {
const char *done = rebase_path_done();
int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666);
int ret = 0;

if (fd >= 0 && offset > prev_offset &&
write_in_full(fd, todo_list->buf.buf + prev_offset,
offset - prev_offset) < 0) {
close(fd);
return error_errno(_("could not write to '%s'"),
done_path);
}
if (fd >= 0)
close(fd);
if (fd < 0)
return 0;
if (write_in_full(fd, get_item_line(todo_list, next - 1),
get_item_line_length(todo_list, next - 1))
< 0)
ret = error_errno(_("could not write to '%s'"), done);
if (close(fd) < 0)
ret = error_errno(_("failed to finalize '%s'"), done);
return ret;
}
return 0;
}
Expand Down Expand Up @@ -3307,8 +3322,7 @@ int skip_unnecessary_picks(void)
oid = &item->commit->object.oid;
}
if (i > 0) {
int offset = i < todo_list.nr ?
todo_list.items[i].offset_in_buf : todo_list.buf.len;
int offset = get_item_line_offset(&todo_list, i);
const char *done_path = rebase_path_done();

fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
Expand Down Expand Up @@ -3488,12 +3502,10 @@ int rearrange_squash(void)
continue;

while (cur >= 0) {
int offset = todo_list.items[cur].offset_in_buf;
int end_offset = cur + 1 < todo_list.nr ?
todo_list.items[cur + 1].offset_in_buf :
todo_list.buf.len;
char *bol = todo_list.buf.buf + offset;
char *eol = todo_list.buf.buf + end_offset;
const char *bol =
get_item_line(&todo_list, cur);
const char *eol =
get_item_line(&todo_list, cur + 1);

/* replace 'pick', by 'fixup' or 'squash' */
command = todo_list.items[cur].command;
Expand Down

0 comments on commit a01c2a5

Please sign in to comment.