Re: [PATCH 2/4] ll-merge: replace flag argument with options struct

Jonathan Nieder [off-list ref] writes:

Keeping track of the flag bits is proving more trouble than it's
worth.  Instead, use a pointer to an options struct like most similar
APIs do.

Callers with no special requests can pass NULL to request the default
options.

Cc: Bert Wesarg <redacted>
Cc: Avery Pennarun <redacted>
Helped-by: Justin Frankel [off-list ref]
Helped-by: Bert Wesarg [off-list ref]
Signed-off-by: Jonathan Nieder <redacted>
---
This time, with updated documentation.

Thanks.

Hmph, two returns...

Fun---expecting tail recursion elimination ;-)?

+	}
+
-	if (flag & LL_OPT_RENORMALIZE) {
+	if (opts->renormalize) {
 		normalize_file(ancestor, path);
 		normalize_file(ours, path);
 		normalize_file(theirs, path);
...
 		}
 	}
 	driver = find_ll_merge_driver(ll_driver_name);

A tangent, as this comment is not about the "richer ll-merge" series.

The above strikes me that the low level merge driver *ought* to have a say
in the use of renormalize.  For example, ll_merge_binary() may probably
not want to have its input renormalized, no?

Junio C Hamano wrote:

Jonathan Nieder [off-list ref] writes:

quoted

+	if (!opts) {
+		struct ll_merge_options default_opts = {0};
+		return ll_merge(result_buf, path, ancestor, ancestor_label,
+				ours, our_label, theirs, their_label,
+				&default_opts);

Fun---expecting tail recursion elimination ;-)?

Fun but not warranted.  Let's simplify.

-- 8< --
Subject: ll-merge: simplify opts == NULL case

As long as sizeof(struct ll_merge_options) is small, there is not
much reason not to keep a copy of the default merge options in the BSS
section.  In return, we get clearer code and one less stack frame in
the opts == NULL case.

Signed-off-by: Jonathan Nieder <redacted>
---
 ll-merge.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/ll-merge.c b/ll-merge.c
index 007dd3e..6ce512e 100644
--- a/ll-merge.c
+++ b/ll-merge.c

@@ -351,16 +351,13 @@ int ll_merge(mmbuffer_t *result_buf,
 	     const struct ll_merge_options *opts)
 {
 	static struct git_attr_check check[2];
+	static const struct ll_merge_options default_opts;
 	const char *ll_driver_name = NULL;
 	int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
 	const struct ll_merge_driver *driver;
 
-	if (!opts) {
-		struct ll_merge_options default_opts = {0};
-		return ll_merge(result_buf, path, ancestor, ancestor_label,
-				ours, our_label, theirs, their_label,
-				&default_opts);
-	}
+	if (!opts)
+		opts = &default_opts;
 
 	if (opts->renormalize) {
 		normalize_file(ancestor, path);

-- 
1.7.4.rc2