[v2,13/13] name-rev: cleanup name_ref()
diff mbox series

Message ID 20191112103821.30265-14-szeder.dev@gmail.com
State New
Headers show
Series
  • name-rev: eliminate recursion
Related show

Commit Message

SZEDER Gábor Nov. 12, 2019, 10:38 a.m. UTC
Earlier patches in this series moved a couple of conditions from the
recursive name_rev() function into its caller name_ref(), for no other
reason than to make eliminating the recursion a bit easier to follow.

Since the previous patch name_rev() is not recursive anymore, so let's
move all those conditions back into name_rev().

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 builtin/name-rev.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Jonathan Tan Nov. 27, 2019, 6:01 p.m. UTC | #1
> Earlier patches in this series moved a couple of conditions from the
> recursive name_rev() function into its caller name_ref(), for no other
> reason than to make eliminating the recursion a bit easier to follow.
> 
> Since the previous patch name_rev() is not recursive anymore, so let's
> move all those conditions back into name_rev().

I don't really see the need for this code movement, to be honest. There
is no big difference in doing the checks in one place or the other, and
if you ask me, it might even be better to do it in the caller of
name_rev(), and leave name_rev() to handle only the naming.

Patch
diff mbox series

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a3b796eac4..cc488ee319 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -107,12 +107,26 @@  static struct rev_name *create_or_update_name(struct commit *commit,
 
 static void name_rev(struct commit *start_commit,
 		const char *tip_name, timestamp_t taggerdate,
-		int from_tag)
+		int from_tag, int deref)
 {
 	struct prio_queue queue;
 	struct commit *commit;
 	struct commit **parents_to_queue = NULL;
 	size_t parents_to_queue_nr, parents_to_queue_alloc = 0;
+	char *to_free = NULL;
+
+	parse_commit(start_commit);
+	if (start_commit->date < cutoff)
+		return;
+
+	if (deref)
+		tip_name = to_free = xstrfmt("%s^0", tip_name);
+
+	if (!create_or_update_name(start_commit, tip_name, taggerdate, 0, 0,
+				   from_tag)) {
+		free(to_free);
+		return;
+	}
 
 	memset(&queue, 0, sizeof(queue)); /* Use the prio_queue as LIFO */
 	prio_queue_put(&queue, start_commit);
@@ -309,20 +323,7 @@  static int name_ref(const char *path, const struct object_id *oid, int flags, vo
 		if (taggerdate == TIME_MAX)
 			taggerdate = commit->date;
 		path = name_ref_abbrev(path, can_abbreviate_output);
-		if (commit->date >= cutoff) {
-			const char *tip_name;
-			char *to_free = NULL;
-			if (deref)
-				tip_name = to_free = xstrfmt("%s^0", path);
-			else
-				tip_name = xstrdup(path);
-			if (create_or_update_name(commit, tip_name, taggerdate,
-						  0, 0, from_tag))
-				name_rev(commit, tip_name, taggerdate,
-					 from_tag);
-			else
-				free(to_free);
-		}
+		name_rev(commit, xstrdup(path), taggerdate, from_tag, deref);
 	}
 	return 0;
 }