Re: [PATCH 07/15] sequencer: lib'ify read_populate_opts()

Johannes Schindelin [off-list ref] writes:

-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;

This may not be sufficient to avoid die(), unless we know that the
file we are reading is syntactically sound.  git_config_from_file()
will die in config.c::git_parse_source() when the config_source sets
die_on_error, and it is set in config.c::do_config_from_file().

The source we are reading from is created when the sequencer
machinery starts and is only written by save_opts() which is
called by the sequencer machinery using git_config_set*() calls,
so I think it is OK to assume that we won't hit errors that would
cause git_config_from_file() to die, at least for now.

Hi Junio,

On Fri, 26 Aug 2016, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

-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;

This may not be sufficient to avoid die(), unless we know that the
file we are reading is syntactically sound.  git_config_from_file()
will die in config.c::git_parse_source() when the config_source sets
die_on_error, and it is set in config.c::do_config_from_file().

The source we are reading from is created when the sequencer
machinery starts and is only written by save_opts() which is
called by the sequencer machinery using git_config_set*() calls,
so I think it is OK to assume that we won't hit errors that would
cause git_config_from_file() to die, at least for now.

I amended the commit message.

Ciao,
Dscho