[PATCH 4/5] diff: refactor init/release API

["Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>]

This patch:

 - renames diff_setup() to init_diff()
 - gets rid of diff_tree_setup_paths()
 - renames diff_tree_release_paths() to release_diff()

The first one makes it probably just personal taste. I find "init"
better name for where input can be garbage, as opposed to "setup",
where the input is at least sane.

release_diff() is supposed to take care of more cleanup stuff as
struct diff_options grows.

diff_tree_release_paths() is not just a simple wrapper around
init_pathspec(). Remove it and manipulate diff_options->pathspec
directly.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/technical/api-diff.txt |    2 +-
 builtin/blame.c                      |   22 +++++++---------------
 builtin/merge.c                      |    2 +-
 builtin/reset.c                      |    4 ++--
 diff-no-index.c                      |    6 +++---
 diff.c                               |    8 +++++++-
 diff.h                               |    5 ++---
 merge-recursive.c                    |    2 +-
 notes-merge.c                        |    8 ++++----
 patch-ids.c                          |    2 +-
 revision.c                           |    4 ++--
 tree-diff.c                          |   21 ++++-----------------
 12 files changed, 35 insertions(+), 51 deletions(-)

diff --git a/Documentation/technical/api-diff.txt b/Documentation/technical/api-diff.txt
index 20b0241..b88ee9e 100644
--- a/Documentation/technical/api-diff.txt
+++ b/Documentation/technical/api-diff.txt
@@ -18,7 +18,7 @@ Calling sequence
 ----------------
 
 * Prepare `struct diff_options` to record the set of diff options, and
-  then call `diff_setup()` to initialize this structure.  This sets up
+  then call `init_diff()` to initialize this structure.  This sets up
   the vanilla default.
 
 * Fill in the options structure to specify desired output format, rename
diff --git a/builtin/blame.c b/builtin/blame.c
index aa30ec5..f58e6e4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -383,14 +383,13 @@ static struct origin *find_origin(struct scoreboard *sb,
 	 * and origin first.  Most of the time they are the
 	 * same and diff-tree is fairly efficient about this.
 	 */
-	diff_setup(&diff_opts);
+	init_diff(&diff_opts);
 	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.detect_rename = 0;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	paths[0] = origin->path;
 	paths[1] = NULL;
-
-	diff_tree_setup_paths(paths, &diff_opts);
+	init_pathspec(&diff_opts.pathspec, paths);
 	if (diff_setup_done(&diff_opts) < 0)
 		die("diff-setup");
 
@@ -441,7 +440,7 @@ static struct origin *find_origin(struct scoreboard *sb,
 		}
 	}
 	diff_flush(&diff_opts);
-	diff_tree_release_paths(&diff_opts);
+	release_diff(&diff_opts);
 	if (porigin) {
 		/*
 		 * Create a freestanding copy that is not part of
@@ -469,15 +468,12 @@ static struct origin *find_rename(struct scoreboard *sb,
 	struct origin *porigin = NULL;
 	struct diff_options diff_opts;
 	int i;
-	const char *paths[2];
 
-	diff_setup(&diff_opts);
+	init_diff(&diff_opts);
 	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = origin->path;
-	paths[0] = NULL;
-	diff_tree_setup_paths(paths, &diff_opts);
 	if (diff_setup_done(&diff_opts) < 0)
 		die("diff-setup");
 
@@ -500,7 +496,7 @@ static struct origin *find_rename(struct scoreboard *sb,
 		}
 	}
 	diff_flush(&diff_opts);
-	diff_tree_release_paths(&diff_opts);
+	release_diff(&diff_opts);
 	return porigin;
 }
 
@@ -1048,7 +1044,6 @@ static int find_copy_in_parent(struct scoreboard *sb,
 			       int opt)
 {
 	struct diff_options diff_opts;
-	const char *paths[1];
 	int i, j;
 	int retval;
 	struct blame_list *blame_list;
@@ -1058,12 +1053,9 @@ static int find_copy_in_parent(struct scoreboard *sb,
 	if (!blame_list)
 		return 1; /* nothing remains for this target */
 
-	diff_setup(&diff_opts);
+	init_diff(&diff_opts);
 	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
-
-	paths[0] = NULL;
-	diff_tree_setup_paths(paths, &diff_opts);
 	if (diff_setup_done(&diff_opts) < 0)
 		die("diff-setup");
 
@@ -1147,7 +1139,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 	}
 	reset_scanned_flag(sb);
 	diff_flush(&diff_opts);
-	diff_tree_release_paths(&diff_opts);
+	release_diff(&diff_opts);
 	return retval;
 }
 
diff --git a/builtin/merge.c b/builtin/merge.c
index a89ddbb..c74ed1f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -379,7 +379,7 @@ static void finish(const unsigned char *new_head, const char *msg)
 	}
 	if (new_head && show_diffstat) {
 		struct diff_options opts;
-		diff_setup(&opts);
+		init_diff(&opts);
 		opts.output_format |=
 			DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
 		opts.detect_rename = DIFF_DETECT_RENAME;
diff --git a/builtin/reset.c b/builtin/reset.c
index 5de2bce..36b0605 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -195,10 +195,10 @@ static int read_from_tree(const char *prefix, const char **argv,
 	struct diff_options opt;
 
 	memset(&opt, 0, sizeof(opt));
-	diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &index_was_discarded;
+	init_pathspec(&opt.pathspec, get_pathspec(prefix, (const char **)argv));
 
 	index_fd = hold_locked_index(lock, 1);
 	index_was_discarded = 0;
@@ -207,7 +207,7 @@ static int read_from_tree(const char *prefix, const char **argv,
 		return 1;
 	diffcore_std(&opt);
 	diff_flush(&opt);
-	diff_tree_release_paths(&opt);
+	release_diff(&opt);
 
 	if (!index_was_discarded)
 		/* The index is still clobbered from do_diff_cache() */
diff --git a/diff-no-index.c b/diff-no-index.c
index 3a36144..a60eda7 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -203,7 +203,7 @@ void diff_no_index(struct rev_info *revs,
 		usagef("git diff %s <path> <path>",
 		       no_index ? "--no-index" : "[--no-index]");
 
-	diff_setup(&revs->diffopt);
+	init_diff(&revs->diffopt);
 	for (i = 1; i < argc - 2; ) {
 		int j;
 		if (!strcmp(argv[i], "--no-index"))
@@ -245,10 +245,10 @@ void diff_no_index(struct rev_info *revs,
 			     : p);
 			paths[i] = p;
 		}
-		diff_tree_setup_paths(paths, &revs->diffopt);
+		init_pathspec(&revs->diffopt.pathspec, paths);
 	}
 	else
-		diff_tree_setup_paths(argv + argc - 2, &revs->diffopt);
+		init_pathspec(&revs->diffopt.pathspec, argv + argc - 2);
 	revs->diffopt.skip_stat_unmatch = 1;
 	if (!revs->diffopt.output_format)
 		revs->diffopt.output_format = DIFF_FORMAT_PATCH;
diff --git a/diff.c b/diff.c
index 6640857..6f206a9 100644
--- a/diff.c
+++ b/diff.c
@@ -2847,7 +2847,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 	builtin_checkdiff(name, other, attr_path, p->one, p->two, o);
 }
 
-void diff_setup(struct diff_options *options)
+void init_diff(struct diff_options *options)
 {
 	memcpy(options, &default_diff_options, sizeof(*options));
 
@@ -2976,6 +2976,12 @@ int diff_setup_done(struct diff_options *options)
 	return 0;
 }
 
+
+void release_diff(struct diff_options *o)
+{
+	free_pathspec(&o->pathspec);
+}
+
 static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
 {
 	char c, *eq;
diff --git a/diff.h b/diff.h
index 310bd6b..12a9907 100644
--- a/diff.h
+++ b/diff.h
@@ -160,8 +160,6 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix);
 
 extern const char mime_boundary_leader[];
 
-extern void diff_tree_setup_paths(const char **paths, struct diff_options *);
-extern void diff_tree_release_paths(struct diff_options *);
 extern int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 		     const char *base, struct diff_options *opt);
 extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,
@@ -226,9 +224,10 @@ extern int parse_long_opt(const char *opt, const char **argv,
 extern int git_diff_basic_config(const char *var, const char *value, void *cb);
 extern int git_diff_ui_config(const char *var, const char *value, void *cb);
 extern int diff_use_color_default;
-extern void diff_setup(struct diff_options *);
+extern void init_diff(struct diff_options *);
 extern int diff_opt_parse(struct diff_options *, const char **, int);
 extern int diff_setup_done(struct diff_options *);
+extern void release_diff(struct diff_options *);
 
 #define DIFF_DETECT_RENAME	1
 #define DIFF_DETECT_COPY	2
diff --git a/merge-recursive.c b/merge-recursive.c
index 16c2dbe..bbf48c9 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -429,7 +429,7 @@ static struct string_list *get_renames(struct merge_options *o,
 	struct diff_options opts;
 
 	renames = xcalloc(1, sizeof(struct string_list));
-	diff_setup(&opts);
+	init_diff(&opts);
 	DIFF_OPT_SET(&opts, RECURSIVE);
 	opts.detect_rename = DIFF_DETECT_RENAME;
 	opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
diff --git a/notes-merge.c b/notes-merge.c
index 1467ad3..acf98e5 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -131,7 +131,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 	trace_printf("\tdiff_tree_remote(base = %.7s, remote = %.7s)\n",
 	       sha1_to_hex(base), sha1_to_hex(remote));
 
-	diff_setup(&opt);
+	init_diff(&opt);
 	DIFF_OPT_SET(&opt, RECURSIVE);
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	if (diff_setup_done(&opt) < 0)
@@ -178,7 +178,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 		       sha1_to_hex(mp->remote));
 	}
 	diff_flush(&opt);
-	diff_tree_release_paths(&opt);
+	release_diff(&opt);
 
 	*num_changes = len;
 	return changes;
@@ -195,7 +195,7 @@ static void diff_tree_local(struct notes_merge_options *o,
 	trace_printf("\tdiff_tree_local(len = %i, base = %.7s, local = %.7s)\n",
 	       len, sha1_to_hex(base), sha1_to_hex(local));
 
-	diff_setup(&opt);
+	init_diff(&opt);
 	DIFF_OPT_SET(&opt, RECURSIVE);
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	if (diff_setup_done(&opt) < 0)
@@ -265,7 +265,7 @@ static void diff_tree_local(struct notes_merge_options *o,
 		       sha1_to_hex(mp->local));
 	}
 	diff_flush(&opt);
-	diff_tree_release_paths(&opt);
+	release_diff(&opt);
 }
 
 static void check_notes_merge_worktree(struct notes_merge_options *o)
diff --git a/patch-ids.c b/patch-ids.c
index 5717257..e8ff156 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -37,7 +37,7 @@ struct patch_id_bucket {
 int init_patch_ids(struct patch_ids *ids)
 {
 	memset(ids, 0, sizeof(*ids));
-	diff_setup(&ids->diffopts);
+	init_diff(&ids->diffopts);
 	DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
 	if (diff_setup_done(&ids->diffopts) < 0)
 		return error("diff_setup_done failed");
diff --git a/revision.c b/revision.c
index f9f66de..014b723 100644
--- a/revision.c
+++ b/revision.c
@@ -925,7 +925,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->grep_filter.header_tail = &(revs->grep_filter.header_list);
 	revs->grep_filter.regflags = REG_NEWLINE;
 
-	diff_setup(&revs->diffopt);
+	init_diff(&revs->diffopt);
 	if (prefix && !revs->diffopt.prefix) {
 		revs->diffopt.prefix = prefix;
 		revs->diffopt.prefix_length = strlen(prefix);
@@ -1662,7 +1662,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	revs->diffopt.abbrev = revs->abbrev;
 	if (diff_setup_done(&revs->diffopt) < 0)
 		die("diff_setup_done failed");
-	diff_tree_setup_paths(revs->diffopt.pathspec.raw, &revs->pruning);
+	init_pathspec(&revs->pruning.pathspec, revs->diffopt.pathspec.raw);
 
 	compile_grep_patterns(&revs->grep_filter);
 
diff --git a/tree-diff.c b/tree-diff.c
index 3954281..7df9064 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -209,26 +209,23 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	struct diff_options diff_opts;
 	struct diff_queue_struct *q = &diff_queued_diff;
 	struct diff_filepair *choice;
-	const char *paths[1];
 	int i;
 
 	/* Remove the file creation entry from the diff queue, and remember it */
 	choice = q->queue[0];
 	q->nr = 0;
 
-	diff_setup(&diff_opts);
+	init_diff(&diff_opts);
 	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->pathspec.raw[0];
 	diff_opts.break_opt = opt->break_opt;
-	paths[0] = NULL;
-	diff_tree_setup_paths(paths, &diff_opts);
 	if (diff_setup_done(&diff_opts) < 0)
 		die("unable to set up diff options to follow renames");
 	diff_tree(t1, t2, base, &diff_opts);
 	diffcore_std(&diff_opts);
-	diff_tree_release_paths(&diff_opts);
+	release_diff(&diff_opts);
 
 	/* Go through the new set of filepairing, and see if we find a more interesting one */
 	opt->found_follow = 0;
@@ -247,9 +244,9 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 			choice = p;
 
 			/* Update the path we use from now on.. */
-			diff_tree_release_paths(opt);
+			release_diff(opt);
 			opt->pathspec.raw[0] = xstrdup(p->one->path);
-			diff_tree_setup_paths(opt->pathspec.raw, opt);
+			init_pathspec(&opt->pathspec, opt->pathspec.raw);
 
 			/*
 			 * The caller expects us to return a set of vanilla
@@ -323,13 +320,3 @@ int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_
 	free(tree);
 	return retval;
 }
-
-void diff_tree_release_paths(struct diff_options *opt)
-{
-	free_pathspec(&opt->pathspec);
-}
-
-void diff_tree_setup_paths(const char **p, struct diff_options *opt)
-{
-	init_pathspec(&opt->pathspec, p);
-}
-- 
1.7.3.1.256.g2539c.dirty