Re: [PATCH 11/18] revert: Save command-line options for continuing operation

On Thursday 28 July 2011 18:52:24 Ramkumar Ramachandra wrote:

+static void save_opts(struct replay_opts *opts)
+{
+	const char *opts_file = git_path(SEQ_OPTS_FILE);
+	struct strbuf buf = STRBUF_INIT;
+	int i;
+
+	if (opts->no_commit)
+		git_config_set_in_file(opts_file, "options.no-commit", "true");
+	if (opts->edit)
+		git_config_set_in_file(opts_file, "options.edit", "true");
+	if (opts->signoff)
+		git_config_set_in_file(opts_file, "options.signoff", "true");
+	if (opts->record_origin)
+		git_config_set_in_file(opts_file, "options.record-origin", "true");
+	if (opts->allow_ff)
+		git_config_set_in_file(opts_file, "options.allow-ff", "true");
+	if (opts->mainline) {
+		strbuf_reset(&buf);

It is not necessary to reset &buf here.

+		strbuf_addf(&buf, "%d", opts->mainline);
+		git_config_set_in_file(opts_file, "options.mainline", buf.buf);
+	}

And perhaps it would be clearer if it was:

+       if (opts->mainline) {
+               struct strbuf buf = STRBUF_INIT;
+               strbuf_addf(&buf, "%d", opts->mainline);
+               git_config_set_in_file(opts_file, "options.mainline", buf.buf);
+               strbuf_release(&buf);
+       }

+	if (opts->strategy)
+		git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
+	if (opts->xopts) {

Other nit: maybe you could put "int i" here, instead of at the beginning of 
the function.

+		for (i = 0; i < opts->xopts_nr; i++)
+			git_config_set_multivar_in_file(opts_file,
+							"options.strategy-option",
+							opts->xopts[i], "^$", 0);
+	}
+
+	strbuf_release(&buf);
+}

Thanks,
Christian.