Re: [PATCH 11/15] sequencer: lib'ify save_todo()
From: Eric Sunshine <hidden>
Date: 2016-08-24 07:38:36
On Tue, Aug 23, 2016 at 12:07 PM, Johannes Schindelin [off-list ref] wrote:
quoted hunk ↗ jump to hunk
To be truly useful, the sequencer should never die() but always return an error. Signed-off-by: Johannes Schindelin <redacted> ---diff --git a/sequencer.c b/sequencer.c@@ -929,24 +929,29 @@ static int sequencer_rollback(struct replay_opts *opts) -static void save_todo(struct commit_list *todo_list, struct replay_opts *opts) +static int save_todo(struct commit_list *todo_list, struct replay_opts *opts) { static struct lock_file todo_lock; struct strbuf buf = STRBUF_INIT; int fd; - fd = hold_lock_file_for_update(&todo_lock, git_path_todo_file(), LOCK_DIE_ON_ERROR); + fd = hold_lock_file_for_update(&todo_lock, git_path_todo_file(), 0); + if (fd < 0) + return error_errno(_("Could not lock '%s'"), + git_path_todo_file()); if (format_todo(&buf, todo_list, opts) < 0) - die(_("Could not format %s."), git_path_todo_file()); + return error(_("Could not format %s."), git_path_todo_file());
format_todo() doesn't seem to make any promises about the state of the strbuf upon error, so should this be releasing the strbuf before returning?
if (write_in_full(fd, buf.buf, buf.len) < 0) {
strbuf_release(&buf);
- die_errno(_("Could not write to %s"), git_path_todo_file());
+ return error_errno(_("Could not write to %s"),
+ git_path_todo_file());Do the above two new error returns need to rollback the lockfile?
}
if (commit_lock_file(&todo_lock) < 0) {
strbuf_release(&buf);
- die(_("Error wrapping up %s."), git_path_todo_file());
+ return error(_("Error wrapping up %s."), git_path_todo_file());
}
strbuf_release(&buf);
+ return 0;
}