Re: [PATCH v8 6/8] submodule: refactor show_submodule_summary with helper function
From: Junio C Hamano <hidden>
Date: 2016-08-19 20:24:29
Jacob Keller [off-list ref] writes:
quoted hunk
@@ -290,12 +289,6 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path, add_pending_object(rev, &left->object, path); add_pending_object(rev, &right->object, path); 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; - } for (list = merge_bases; list; list = list->next) { list->item->object.flags |= UNINTERESTING; add_pending_object(rev, &list->item->object,
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.