Subject: Re: [PATCH] maintenance: fix two memory leaks

From: Junio C Hamano <[email-masked]>
Date: Mon, 10 May 2021 16:11:59 +0900

"[email-masked]" <[email-masked]> writes:

>>@@ -1999,6 +2000,7 @@ static int update_background_schedule(int enable)
>> die("unknown background scheduler: %s", scheduler);
>>
>> rollback_lock_file(&lk);
>>+	free(lock_path); 
> Based on your change, I think when "hold_lock_file_for_update()<0", we should also free local_path
> Thanks

Meaning something like this?

 builtin/gc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git c/builtin/gc.c w/builtin/gc.c
index 98a803196b..50565c37c7 100644
--- c/builtin/gc.c
+++ w/builtin/gc.c
@@ -1971,8 +1971,10 @@ static int update_background_schedule(int enable)
 		cmd = sep + 1;
 	}
 
-	if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0)
-		return error(_("another process is scheduling background maintenance"));
+	if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) {
+		result = error(_("another process is scheduling background maintenance"));
+		goto cleanup;
+	}
 
 	if (!strcmp(scheduler, "launchctl"))
 		result = launchctl_update_schedule(enable, get_lock_file_fd(&lk), cmd);
@@ -1984,6 +1986,9 @@ static int update_background_schedule(int enable)
 		die("unknown background scheduler: %s", scheduler);
 
 	rollback_lock_file(&lk);
+
+cleanup:
+	free(lock_path);
 	free(testing);
 	return result;
 }