builtin/fetch-pack: cleanup before return error
PATCH 1/1 builtin/fetch-pack: cleanup before return error
From: Lidong Yan <[email protected]>
In builtin/fetch-pack.c:cmd_fetch_pack(), if finish_connect() failed, it returns error code without cleanup which cause memory leak. Add cleanup label before frees in the end of cmd_fetch_pack(), and add goto cleanup if finish_connect() failed.
---
builtin/fetch-pack: cleanup before return error
In builtin/fetch-pack.c:cmd_fetch_pack(), if finish_connect() failed, it returns error code without cleanup which cause memory leak. Add cleanup label before frees in the end of cmd_fetch_pack(), and add goto cleanup if finish_connect() failed.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1990%2Fbrandb97%2Ffix-cmd-fetch-pack-leak-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1990/brandb97/fix-cmd-fetch-pack-leak-v1
Pull-Request: https://github.com/git/git/pull/1990
builtin/fetch-pack.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
builtin/fetch-pack.c +5 -2
@@ -274,8 +274,10 @@ int cmd_fetch_pack(int argc, }close (fd[0 ]);close (fd[1 ]); -if (finish_connect (conn)) -return 1 ; +if (finish_connect (conn)) { + ret =1 ; + goto cleanup; + } ret = !fetched_refs;
@@ -291,6 +293,7 @@ int cmd_fetch_pack(int argc,printf ("%s %s\n" ,oid_to_hex (&ref->old_oid), ref->name); +cleanup:for (size_t i =0 ; i < nr_sought; i++)free_one_ref (sought_to_free[i]);free (sought_to_free);
base-commit: 7014b55638da979331baf8dc31c4e1d697cf2d67
Signature · 1 line
gitgitgadget
Quoted 18 lines · Lidong Yan via GitGitGadget
builtin/fetch-pack.c +4
-2@@ -274,8 +274,10 @@ int cmd_fetch_pack(int argc, }close (fd[0 ]);close (fd[1 ]); -if (finish_connect (conn)) -return 1 ; +if (finish_connect (conn)) { + ret =1 ; + goto cleanup; + } ret = !fetched_refs;
builtin/fetch-pack.c int cmd_fetch_pack(int argc, · line 299
Quoted 7 lines
builtin/fetch-pack.c +1
-0@@ -291,6 +293,7 @@ int cmd_fetch_pack(int argc,printf ("%s %s\n" ,oid_to_hex (&ref->old_oid), ref->name); +cleanup:for (size_t i =0 ; i < nr_sought; i++)free_one_ref (sought_to_free[i]);free (sought_to_free);
Sensible again. My only worry is that some of the variables that get free'd here may not have been initialized. But I went through all of them and all of them are NULL- or zero-initialized indeed, so this looks good to me. Thanks!
Patrick