Re: [PATCH 11/15] sequencer: lib'ify save_todo()
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Aug 24, 2016, 07:36 UTC
- Message-ID
- <CAPig+cQ2xH1ytsTGNDeDZiHTZpnp28XUJoAY+W-+iw+85gMbtg@mail.gmail.com>
- In-Reply-To
- <fff4cdfa36355af7917a06be9f67b50b56bfce99.1471968378.git.johannes.schindelin@gmx.de>
On Tue, Aug 23, 2016 at 12:07 PM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
Show 22 quoted lines
> To be truly useful, the sequencer should never die() but always return
> an error.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 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?
Show 5 quoted lines
> 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?
Show 9 quoted lines
> }
> 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;
> }