Re: [PATCH 06/15] sequencer: lib'ify read_populate_todo()

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 24, 2016, 07:24 UTC
Message-ID: <CAPig+cSR8BHFdMJz5mFkpJ3UU6w0Xmjav+tu6f3DrkG4Gi-v6A@mail.gmail.com>
In-Reply-To: <0de75bbce8ade0c6e5cf87d3647faa71a89c6275.1471968378.git.johannes.schindelin@gmx.de>

On Tue, Aug 23, 2016 at 12:07 PM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

Show 13 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
> @@ -754,18 +754,21 @@ static void read_populate_todo(struct commit_list **todo_list,
>
>         fd = open(git_path_todo_file(), O_RDONLY);
>         if (fd < 0)
> -               die_errno(_("Could not open %s"), git_path_todo_file());
> +               return error(_("Could not open %s (%s)"),
> +                       git_path_todo_file(), strerror(errno));

error_errno() perhaps?

Show 14 quoted lines

>         if (strbuf_read(&buf, fd, 0) < 0) {
>                 close(fd);
>                 strbuf_release(&buf);
> -               die(_("Could not read %s."), git_path_todo_file());
> +               return error(_("Could not read %s."), git_path_todo_file());
>         }
>         close(fd);
>
>         res = parse_insn_buffer(buf.buf, todo_list, opts);
>         strbuf_release(&buf);
>         if (res)
> -               die(_("Unusable instruction sheet: %s"), git_path_todo_file());
> +               return error(_("Unusable instruction sheet: %s"),
> +                       git_path_todo_file());

Neither 'fd' nor 'buf' are leaked by these two new error returns. Good.

> +       return 0;
>  }