walker_fetch: fix minor memory leak

We sometimes allocate "msg" on the heap, but will fail to
free it if we hit the failure code path. We can instead keep
a separate variable that is safe to be freed no matter how
we get to the failure code path.

While we're here, we can also do two readability
improvements:

  1. Use xstrfmt instead of a manual malloc/sprintf

  2. Due to the "maybe we allocate msg, maybe we don't"
     strategy, the logic for deciding which message to show
     was split into two parts. Since the deallocation is now
     pushed onto a separate variable, this is no longer a
     concern, and we can keep all of the logic in the same
     place.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 9 insertions, 9 deletions

diff --git a/walker.c b/walker.c
index 1dd86b8f3..014826464 100644
--- a/walker.c
+++ b/walker.c
@@ -253,7 +253,8 @@ int walker_fetch(struct walker *walker, int targets, char **target,
 {
 	struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *));
 	unsigned char *sha1 = xmalloc(targets * 20);
-	char *msg;
+	const char *msg;
+	char *to_free = NULL;
 	int ret;
 	int i;
 
@@ -285,21 +286,19 @@ int walker_fetch(struct walker *walker, int targets, char **target,
 	if (loop(walker))
 		goto unlock_and_fail;
 
-	if (write_ref_log_details) {
-		msg = xmalloc(strlen(write_ref_log_details) + 12);
-		sprintf(msg, "fetch from %s", write_ref_log_details);
-	} else {
-		msg = NULL;
-	}
+	if (write_ref_log_details)
+		msg = to_free = xstrfmt("fetch from %s", write_ref_log_details);
+	else
+		msg = "fetch (unknown)";
 	for (i = 0; i < targets; i++) {
 		if (!write_ref || !write_ref[i])
 			continue;
-		ret = write_ref_sha1(lock[i], &sha1[20 * i], msg ? msg : "fetch (unknown)");
+		ret = write_ref_sha1(lock[i], &sha1[20 * i], msg);
 		lock[i] = NULL;
 		if (ret)
 			goto unlock_and_fail;
 	}
-	free(msg);
+	free(to_free);
 
 	return 0;
 
@@ -307,6 +306,7 @@ unlock_and_fail:
 	for (i = 0; i < targets; i++)
 		if (lock[i])
 			unlock_ref(lock[i]);
+	free(to_free);
 
 	return -1;
 }