{"thread":{"id":"11750","subject":"[Resend PATCH] Fix some memory leaks in various places","startedAt":"2008-01-26T10:50:27Z","lastAt":"2008-01-27T02:18:36Z","messageCount":2,"participants":["Mike Hommey","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"66632","messageId":"1201344627-21609-1-git-send-email-mh@glandium.org","threadId":"11750","inReplyTo":null,"subject":"[Resend PATCH] Fix some memory leaks in various places","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-01-26T10:50:27Z","receivedAt":"2008-01-26T10:50:27Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n\n I think I sent this already last month, but since I see that nowhere on\n next or master, I'm resending, just in case it got lost.\n\n builtin-init-db.c |    1 +\n http-walker.c     |   10 ++++++++++\n walker.c          |    2 ++\n 3 files changed, 13 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex e1393b8..df61758 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -415,6 +415,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \tsafe_create_dir(path, 1);\n \tstrcpy(path+len, \"/info\");\n \tsafe_create_dir(path, 1);\n+\tfree(path);\n \n \tif (shared_repository) {\n \t\tchar buf[10];\ndiff --git a/http-walker.c b/http-walker.c\nindex 2c37868..1a02f86 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -231,6 +231,8 @@ static void finish_object_request(struct object_request *obj_req)\n {\n \tstruct stat st;\n \n+\tfree(obj_req->url);\n+\n \tfchmod(obj_req->local, 0444);\n \tclose(obj_req->local); obj_req->local = -1;\n \n@@ -897,9 +899,17 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)\n static void cleanup(struct walker *walker)\n {\n \tstruct walker_data *data = walker->data;\n+\tstruct alt_base *prev_altbase, *altbase = data->alt;\n+\twhile (altbase) {\n+\t\tfree(altbase->base);\n+\t\tprev_altbase = altbase;\n+\t\taltbase = altbase->next;\n+\t\tfree(prev_altbase);\n+\t}\n \thttp_cleanup();\n \n \tcurl_slist_free_all(data->no_pragma_header);\n+\tfree(data);\n }\n \n struct walker *get_http_walker(const char *url)\ndiff --git a/walker.c b/walker.c\nindex adc3e80..64fc419 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -299,6 +299,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\t\tgoto unlock_and_fail;\n \t}\n \tfree(msg);\n+\tfree(sha1);\n \n \treturn 0;\n \n@@ -306,6 +307,7 @@ unlock_and_fail:\n \tfor (i = 0; i < targets; i++)\n \t\tif (lock[i])\n \t\t\tunlock_ref(lock[i]);\n+\tfree(sha1);\n \n \treturn -1;\n }\n-- \n1.5.4.rc3.30.gb084e\n"},{"id":"66655","messageId":"7v4pd0yps3.fsf@gitster.siamese.dyndns.org","threadId":"11750","inReplyTo":"1201344627-21609-1-git-send-email-mh@glandium.org","subject":"Re: [Resend PATCH] Fix some memory leaks in various places","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-27T02:18:36Z","receivedAt":"2008-01-27T02:18:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n> diff --git a/builtin-init-db.c b/builtin-init-db.c\n> index e1393b8..df61758 100644\n> --- a/builtin-init-db.c\n> +++ b/builtin-init-db.c\n> @@ -415,6 +415,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n>  \tsafe_create_dir(path, 1);\n>  \tstrcpy(path+len, \"/info\");\n>  \tsafe_create_dir(path, 1);\n> +\tfree(path);\n>  \n>  \tif (shared_repository) {\n>  \t\tchar buf[10];\n\nThis is \"technically correct but do we care?\" category\nimmediately before exiting.\n\n> diff --git a/http-walker.c b/http-walker.c\n> index 2c37868..1a02f86 100644\n> --- a/http-walker.c\n> +++ b/http-walker.c\n> @@ -231,6 +231,8 @@ static void finish_object_request(struct object_request *obj_req)\n>  {\n>  \tstruct stat st;\n>  \n> +\tfree(obj_req->url);\n> +\n>  \tfchmod(obj_req->local, 0444);\n>  \tclose(obj_req->local); obj_req->local = -1;\n>  \n\nI am not sure about this.  Instead of freeing this memory early,\nthe function's sole caller, process_object_response(), may be\nthe right place to do this after calling this function.\n\nWho is responsible for reclaiming the memory for obj_req itself\nthat is passed to the callback function process_object_response()?\nHaving two different functions release_object_request() and\nfinish_object_request() that clean things up differently makes\nme feel somewhat queasy.\n\n> @@ -897,9 +899,17 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)\n>  static void cleanup(struct walker *walker)\n>  {\n>  \tstruct walker_data *data = walker->data;\n> +\tstruct alt_base *prev_altbase, *altbase = data->alt;\n> +\twhile (altbase) {\n> +\t\tfree(altbase->base);\n> +\t\tprev_altbase = altbase;\n> +\t\taltbase = altbase->next;\n> +\t\tfree(prev_altbase);\n> +\t}\n>  \thttp_cleanup();\n>  \n>  \tcurl_slist_free_all(data->no_pragma_header);\n> +\tfree(data);\n>  }\n>  \n>  struct walker *get_http_walker(const char *url)\n\nLooks Ok.\n\n> diff --git a/walker.c b/walker.c\n> index adc3e80..64fc419 100644\n> --- a/walker.c\n> +++ b/walker.c\n> @@ -299,6 +299,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n>  \t\t\tgoto unlock_and_fail;\n>  \t}\n>  \tfree(msg);\n> +\tfree(sha1);\n>  \n>  \treturn 0;\n>  \n> @@ -306,6 +307,7 @@ unlock_and_fail:\n>  \tfor (i = 0; i < targets; i++)\n>  \t\tif (lock[i])\n>  \t\t\tunlock_ref(lock[i]);\n> +\tfree(sha1);\n>  \n>  \treturn -1;\n>  }\n\nThis does not seem to free something that should not get freed,\nbut there is another leak, I think.  If write_ref_log_details is\nset and write_ref_sha1() fails, goto unlock_and_fail will leak\nmsg, won't it?\n"}]}