Re: [PATCH 11/15] sequencer: lib'ify save_todo()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 23, 2016 at 12:07 PM, Johannes Schindelin
<johannes.schindelin@xxxxxx> wrote:
> To be truly useful, the sequencer should never die() but always return
> an error.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
> 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;
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]