On Wed, Mar 23 2022, Junio C Hamano wrote:

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

+static void release_revisions_commit_list(struct rev_info *revs)
+{
+	struct commit_list *commits = revs->commits;
+
+	if (!commits)
+		return;
+	free_commit_list(commits);
+	revs->commits = NULL;
+}

It makes sense to have this as a separate helper, but the original
implementation this was lifted from is much easier to follow than
this version with an unnecessary rewrite, I would think.

quoted

@@ -4080,10 +4090,7 @@ static void create_boundary_commit_list(struct rev_info *revs)
 	 * boundary commits anyway.  (This is what the code has always
 	 * done.)
 	 */
-	if (revs->commits) {
-		free_commit_list(revs->commits);
-		revs->commits = NULL;
-	}
+	release_revisions_commit_list(revs);

IOW

static void release_revisions_commit_list(struct rev_info *revs)
{
	if (revs->commits) {
		free_commit_list(revs->commits);
		revs->commits = NULL;
	}
}

Sure, will change it.

I think the pre-image is preferrable in context, i.e. to have them all
use the same early abort pattern, but not enough to argue the point :)