sequencer: refactor how original todo list lines are accessed
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>
Johannes Schindelin committed
Apr 25, 2018 at 14:28 UTC
a01c2a5f59cc0f28daeab33355ae4a45490ce7e6
1 file changed
+36
-24
sequencer.c
+36
-24
@@ -1871,6 +1871,23 @@ static int count_commands(struct todo_list *todo_list)
1871
return count;
1872
}
1873
1874
+static int get_item_line_offset(struct todo_list *todo_list, int index)
1875
+{
1876
+ return index < todo_list->nr ?
1877
+ todo_list->items[index].offset_in_buf : todo_list->buf.len;
1878
+}
1879
+
1880
+static const char *get_item_line(struct todo_list *todo_list, int index)
1881
+{
1882
+ return todo_list->buf.buf + get_item_line_offset(todo_list, index);
1883
+}
1884
+
1885
+static int get_item_line_length(struct todo_list *todo_list, int index)
1886
+{
1887
+ return get_item_line_offset(todo_list, index + 1)
1888
+ - get_item_line_offset(todo_list, index);
1889
+}
1890
+
1891
static ssize_t strbuf_read_file_or_whine(struct strbuf *sb, const char *path)
1892
{
1893
int fd;
@@ -2250,29 +2267,27 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
2267
fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
2268
if (fd < 0)
2269
return error_errno(_("could not lock '%s'"), todo_path);
2253
- offset = next < todo_list->nr ?
2254
- todo_list->items[next].offset_in_buf : todo_list->buf.len;
2270
+ offset = get_item_line_offset(todo_list, next);
2271
if (write_in_full(fd, todo_list->buf.buf + offset,
2272
todo_list->buf.len - offset) < 0)
2273
return error_errno(_("could not write to '%s'"), todo_path);
2274
if (commit_lock_file(&todo_lock) < 0)
2275
return error(_("failed to finalize '%s'"), todo_path);
2276
2261
- if (is_rebase_i(opts)) {
2262
- const char *done_path = rebase_path_done();
2263
- int fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
2264
- int prev_offset = !next ? 0 :
2265
- todo_list->items[next - 1].offset_in_buf;
2277
+ if (is_rebase_i(opts) && next > 0) {
2278
+ const char *done = rebase_path_done();
2279
+ int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666);
2280
+ int ret = 0;
2281
2267
- if (fd >= 0 && offset > prev_offset &&
2268
- write_in_full(fd, todo_list->buf.buf + prev_offset,
2269
- offset - prev_offset) < 0) {
2270
- close(fd);
2271
- return error_errno(_("could not write to '%s'"),
2272
- done_path);
2273
- }
2274
- if (fd >= 0)
2275
- close(fd);
2282
+ if (fd < 0)
2283
+ return 0;
2284
+ if (write_in_full(fd, get_item_line(todo_list, next - 1),
2285
+ get_item_line_length(todo_list, next - 1))
2286
+ < 0)
2287
+ ret = error_errno(_("could not write to '%s'"), done);
2288
+ if (close(fd) < 0)
2289
+ ret = error_errno(_("failed to finalize '%s'"), done);
2290
+ return ret;
2291
}
2292
return 0;
2293
}
@@ -3307,8 +3322,7 @@ int skip_unnecessary_picks(void)
3322
oid = &item->commit->object.oid;
3323
}
3324
if (i > 0) {
3310
- int offset = i < todo_list.nr ?
3311
- todo_list.items[i].offset_in_buf : todo_list.buf.len;
3325
+ int offset = get_item_line_offset(&todo_list, i);
3326
const char *done_path = rebase_path_done();
3327
3328
fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
@@ -3488,12 +3502,10 @@ int rearrange_squash(void)
3502
continue;
3503
3504
while (cur >= 0) {
3491
- int offset = todo_list.items[cur].offset_in_buf;
3492
- int end_offset = cur + 1 < todo_list.nr ?
3493
- todo_list.items[cur + 1].offset_in_buf :
3494
- todo_list.buf.len;
3495
- char *bol = todo_list.buf.buf + offset;
3496
- char *eol = todo_list.buf.buf + end_offset;
3505
+ const char *bol =
3506
+ get_item_line(&todo_list, cur);
3507
+ const char *eol =
3508
+ get_item_line(&todo_list, cur + 1);
3509
3510
/* replace 'pick', by 'fixup' or 'squash' */
3511
command = todo_list.items[cur].command;