[PATCH 2/1] revision: simplify mark_argv_for_free() callers

[Jeff King <peff@peff.net>]

BTW, this is a small cleanup that I resisted putting into the earlier
commit in order to keep it focused. But maybe worth doing on top?

-- >8 --
Subject: revision: simplify mark_argv_for_free() callers

You do not want to mark an argv element for freeing unless the caller
has given us the free_removed_argv_elements flag. Originally we just
called free() in this case, so each caller checked the flag itself. Now
that we mark them via a helper function, we can push the check down into
the helper. This saves a little bit of duplicated code, but also
hopefully makes the result conceptually simpler.

Every caller but one was already checking this flag. The exception is
setup_revisions_from_strvec(), but it always sets the flag explicitly
(since its whole purpose is managing argv memory). So even though it was
not checking the flag, doing so is OK (it will always be set).

Signed-off-by: Jeff King <peff@peff.net>
---
 revision.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/revision.c b/revision.c
index 7aee96bd8e..59d6372506 100644
--- a/revision.c
+++ b/revision.c
@@ -2317,8 +2317,11 @@ static timestamp_t parse_age(const char *arg)
  * usual const argv array, even when opt->free_removed_argv_elements is set).
  * We cast away the const on their behalf.
  */
-static void mark_argv_for_free(struct rev_info *revs, const char *str)
+static void mark_argv_for_free(const struct setup_revision_opt *opt,
+			       struct rev_info *revs, const char *str)
 {
+	if (!opt || !opt->free_removed_argv_elements)
+		return;
 	if (!str)
 		return;
 	strvec_push_nodup(&revs->argv_to_free, (char *)str);
@@ -2335,8 +2338,7 @@ static void overwrite_argv(int *argc, const char **argv,
 	 * cases around the free() and NULL operations.
 	 */
 	if (*value != argv[*argc]) {
-		if (opt && opt->free_removed_argv_elements)
-			mark_argv_for_free(revs, argv[*argc]);
+		mark_argv_for_free(opt, revs, argv[*argc]);
 		argv[*argc] = *value;
 		*value = NULL;
 	}
@@ -3055,8 +3057,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 			const char *arg = argv[i];
 			if (strcmp(arg, "--"))
 				continue;
-			if (opt && opt->free_removed_argv_elements)
-				mark_argv_for_free(revs, argv[i]);
+			mark_argv_for_free(opt, revs, argv[i]);
 			argv[i] = NULL;
 			argc = i;
 			if (argv[i + 1])
@@ -3260,8 +3261,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	}
 
 	if (argv) {
-		if (opt && opt->free_removed_argv_elements)
-			mark_argv_for_free(revs, argv[left]);
+		mark_argv_for_free(opt, revs, argv[left]);
 		argv[left] = NULL;
 	}
 
@@ -3283,7 +3283,7 @@ void setup_revisions_from_strvec(struct strvec *argv, struct rev_info *revs,
 	ret = setup_revisions(argv->nr, argv->v, revs, opt);
 
 	for (size_t i = ret; i < argv->nr; i++)
-		mark_argv_for_free(revs, argv->v[i]);
+		mark_argv_for_free(opt, revs, argv->v[i]);
 	argv->nr = ret;
 }
 
-- 
2.55.0.1050.g5a46c03bac