Re: [PATCH v8 6/8] submodule: refactor show_submodule_summary with helper function

Jacob Keller [off-list ref] writes:

Not a new issue with this patch, but I wonder if this commit_list is
leaking here.

+	/*
+	 * Warn about missing commits in the submodule project, but only if
+	 * they aren't null.
+	 */
+	if ((!is_null_oid(one) && !*left) ||
+	     (!is_null_oid(two) && !*right))
+		message = "(commits not present)";
+
+	merge_bases = get_merge_bases(*left, *right);
+	if (merge_bases) {
+		if (merge_bases->item == *left)
+			fast_forward = 1;
+		else if (merge_bases->item == *right)
+			fast_backward = 1;
+	}

And probably merge_bases also leaks here.

It is not cheap to compute merge bases, but show_submodule_summary()
makes two calls to get_merge_bases(), one in show_submodule_header()
and then another inside prepare_submodule_summary() to compute
exactly the same set of merge bases.  We somehow need to reduce it
to just one.

On Fri, Aug 19, 2016 at 1:24 PM, Junio C Hamano [off-list ref] wrote:

And probably merge_bases also leaks here.

It is not cheap to compute merge bases, but show_submodule_summary()
makes two calls to get_merge_bases(), one in show_submodule_header()
and then another inside prepare_submodule_summary() to compute
exactly the same set of merge bases.  We somehow need to reduce it
to just one.

I can make show_submodule_headers take another parameter which we
pass, and then pass that into prepare_submodule_summary...?

Thanks,
Jake