Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> Instead of dying there, let the caller high up in the callchain notice
> the error and handle it (by dying, still).
>
> The only caller of read_populate_opts(), sequencer_continue() can
> already return errors, so its caller must be already prepared to
> handle error returns, and with this step, we make it notice an error
> return from this function.
>
> So this is a safe conversion to make read_populate_opts() callable
> from new callers that want it not to die, without changing the
> external behaviour of anything existing.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
> sequencer.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index e11b24f..be6020a 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -808,12 +808,14 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
> return 0;
> }
>
> -static void read_populate_opts(struct replay_opts **opts_ptr)
> +static int read_populate_opts(struct replay_opts **opts)
> {
> if (!file_exists(git_path_opts_file()))
> - return;
> - if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts_ptr) < 0)
> - die(_("Malformed options sheet: %s"), git_path_opts_file());
> + return 0;
> + if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) < 0)
> + return error(_("Malformed options sheet: %s"),
> + git_path_opts_file());
> + return 0;
As discussed, perhaps have a comment immediately before calling
config-from-file that says that the call could die when it is fed a
syntactically broken file, but we ignore it for now because we will
be writing the file we have written, or something?
> }
>
> static int walk_revs_populate_todo(struct commit_list **todo_list,
> @@ -1021,8 +1023,8 @@ static int sequencer_continue(struct replay_opts *opts)
>
> if (!file_exists(git_path_todo_file()))
> return continue_single_pick();
> - read_populate_opts(&opts);
> - if (read_populate_todo(&todo_list, opts))
> + if (read_populate_opts(&opts) ||
> + read_populate_todo(&todo_list, opts))
> return -1;
>
> /* Verify that the conflict has been resolved */