Re: [PATCH 07/15] sequencer: lib'ify read_populate_opts()
Hi Junio, On Fri, 26 Aug 2016, Junio C Hamano wrote: > Johannes Schindelinwrites: > > > -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
Re: [PATCH 07/15] sequencer: lib'ify read_populate_opts()
Johannes Schindelinwrites: > -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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html