Re: [PATCH] maintenance: fix two memory leaks

From: Junio C Hamano <gitster@pobox.com>
Date: May 10, 2021, 07:11 UTC
Message-ID: <xmqqlf8nqczk.fsf@gitster.g>
In-Reply-To: <67ed4ca6b15a11eba8d80024e87935e7@oschina.cn>

"lilinchao@oschina.cn" <lilinchao@oschina.cn> writes:

Show 7 quoted lines

>>@@ -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;
 }