Re: [Resend PATCH] Fix some memory leaks in various places
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:08
Mike Hommey [off-list ref] writes:
quoted hunk
diff --git a/builtin-init-db.c b/builtin-init-db.c index e1393b8..df61758 100644 --- a/builtin-init-db.c +++ b/builtin-init-db.c@@ -415,6 +415,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix) safe_create_dir(path, 1); strcpy(path+len, "/info"); safe_create_dir(path, 1); + free(path); if (shared_repository) { char buf[10];
This is "technically correct but do we care?" category immediately before exiting.
quoted hunk
diff --git a/http-walker.c b/http-walker.c index 2c37868..1a02f86 100644 --- a/http-walker.c +++ b/http-walker.c@@ -231,6 +231,8 @@ static void finish_object_request(struct object_request *obj_req) { struct stat st; + free(obj_req->url); + fchmod(obj_req->local, 0444); close(obj_req->local); obj_req->local = -1;
I am not sure about this. Instead of freeing this memory early, the function's sole caller, process_object_response(), may be the right place to do this after calling this function. Who is responsible for reclaiming the memory for obj_req itself that is passed to the callback function process_object_response()? Having two different functions release_object_request() and finish_object_request() that clean things up differently makes me feel somewhat queasy.
quoted hunk
@@ -897,9 +899,17 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1) static void cleanup(struct walker *walker) { struct walker_data *data = walker->data; + struct alt_base *prev_altbase, *altbase = data->alt; + while (altbase) { + free(altbase->base); + prev_altbase = altbase; + altbase = altbase->next; + free(prev_altbase); + } http_cleanup(); curl_slist_free_all(data->no_pragma_header); + free(data); } struct walker *get_http_walker(const char *url)
Looks Ok.
quoted hunk
diff --git a/walker.c b/walker.c index adc3e80..64fc419 100644 --- a/walker.c +++ b/walker.c@@ -299,6 +299,7 @@ int walker_fetch(struct walker *walker, int targets, char **target, goto unlock_and_fail; } free(msg); + free(sha1); return 0;@@ -306,6 +307,7 @@ unlock_and_fail: for (i = 0; i < targets; i++) if (lock[i]) unlock_ref(lock[i]); + free(sha1); return -1; }
This does not seem to free something that should not get freed, but there is another leak, I think. If write_ref_log_details is set and write_ref_sha1() fails, goto unlock_and_fail will leak msg, won't it?