[PATCH 1/2] Fix XML parser leaks in http-push

Subsystems: the rest

DORMANTno replies

4 messages, 1 author, 2016-06-15 · open the first message on its own page

[PATCH 1/2] Fix XML parser leaks in http-push

From: Mike Hommey <hidden>
Date: 2016-06-15 22:43:58

XML_Parser were never freed. While at it, move the parser initialization to
right before it is needed.

Signed-off-by: Mike Hommey <redacted>
---
 http-push.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/http-push.c b/http-push.c
index a4a9d1c..f5ba850 100644
--- a/http-push.c
+++ b/http-push.c

@@ -1189,8 +1189,6 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	char *ep;
 	char timeout_header[25];
 	struct remote_lock *lock = NULL;
-	XML_Parser parser = XML_ParserCreate(NULL);
-	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
 	struct xml_ctx ctx;

@@ -1250,6 +1248,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result == CURLE_OK) {
+			XML_Parser parser = XML_ParserCreate(NULL);
+			enum XML_Status result;
 			ctx.name = xcalloc(10, 1);
 			ctx.len = 0;
 			ctx.cdata = NULL;

@@ -1268,6 +1268,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 						XML_GetErrorCode(parser)));
 				lock->timeout = -1;
 			}
+			XML_ParserFree(parser);
 		}
 	} else {
 		fprintf(stderr, "Unable to start LOCK request\n");

@@ -1428,8 +1429,6 @@ static void remote_ls(const char *path, int flags,
 	struct slot_results results;
 	struct strbuf in_buffer = STRBUF_INIT;
 	struct buffer out_buffer = { 0, STRBUF_INIT };
-	XML_Parser parser = XML_ParserCreate(NULL);
-	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
 	struct xml_ctx ctx;
 	struct remote_ls_ctx ls;

@@ -1463,6 +1462,8 @@ static void remote_ls(const char *path, int flags,
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result == CURLE_OK) {
+			XML_Parser parser = XML_ParserCreate(NULL);
+			enum XML_Status result;
 			ctx.name = xcalloc(10, 1);
 			ctx.len = 0;
 			ctx.cdata = NULL;

@@ -1481,6 +1482,7 @@ static void remote_ls(const char *path, int flags,
 					XML_ErrorString(
 						XML_GetErrorCode(parser)));
 			}
+			XML_ParserFree(parser);
 		}
 	} else {
 		fprintf(stderr, "Unable to start PROPFIND request\n");

@@ -1512,8 +1514,6 @@ static int locking_available(void)
 	struct slot_results results;
 	struct strbuf in_buffer = STRBUF_INIT;
 	struct buffer out_buffer = { 0, STRBUF_INIT };
-	XML_Parser parser = XML_ParserCreate(NULL);
-	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
 	struct xml_ctx ctx;
 	int lock_flags = 0;

@@ -1538,6 +1538,8 @@ static int locking_available(void)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result == CURLE_OK) {
+			XML_Parser parser = XML_ParserCreate(NULL);
+			enum XML_Status result;
 			ctx.name = xcalloc(10, 1);
 			ctx.len = 0;
 			ctx.cdata = NULL;

@@ -1556,6 +1558,7 @@ static int locking_available(void)
 						XML_GetErrorCode(parser)));
 				lock_flags = 0;
 			}
+			XML_ParserFree(parser);
 		}
 	} else {
 		fprintf(stderr, "Unable to start PROPFIND request\n");

-- 
1.5.3.7

[PATCH 2/2] Fix some memory leaks in various places

From: Mike Hommey <hidden>
Date: 2016-06-15 22:43:58

Signed-off-by: Mike Hommey <redacted>
---
 builtin-init-db.c |    1 +
 http-walker.c     |   10 ++++++++++
 walker.c          |    2 ++
 3 files changed, 13 insertions(+), 0 deletions(-)

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];

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;

@@ -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)

diff --git a/walker.c b/walker.c
index 397b80d..7473e90 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;
 }

-- 
1.5.3.7

[PATCH 3/2] Fix small memory leaks induced by diff_tree_setup_paths

From: Mike Hommey <hidden>
Date: 2016-06-15 22:43:58

Run diff_tree_release_paths in the appropriate places, and add a test to
avoid NULL dereference. Better safe than sorry.

Signed-off-by: Mike Hommey <redacted>
---

 There are a few more occurrences of diff_tree_setup_paths for which I don't
 know the code enough to correctly place the corresponding
 diff_tree_release_paths. Interesting the function existed but was never
 actually used ;)

 builtin-blame.c |    4 +++-
 builtin-reset.c |    1 +
 tree-diff.c     |    4 +++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 523e55a..041901b 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c

@@ -388,6 +388,7 @@ static struct origin *find_origin(struct scoreboard *sb,
 		}
 	}
 	diff_flush(&diff_opts);
+	diff_tree_release_paths(&diff_opts);
 	if (porigin) {
 		/*
 		 * Create a freestanding copy that is not part of

@@ -444,6 +445,7 @@ static struct origin *find_rename(struct scoreboard *sb,
 		}
 	}
 	diff_flush(&diff_opts);
+	diff_tree_release_paths(&diff_opts);
 	return porigin;
 }

@@ -1167,7 +1169,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 		}
 	}
 	diff_flush(&diff_opts);
-
+	diff_tree_release_paths(&diff_opts);
 	return retval;
 }

diff --git a/builtin-reset.c b/builtin-reset.c
index 4c61025..713c2d5 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c

@@ -158,6 +158,7 @@ static int read_from_tree(const char *prefix, const char **argv,
 		return 1;
 	diffcore_std(&opt);
 	diff_flush(&opt);
+	diff_tree_release_paths(&opt);
 
 	if (!index_was_discarded)
 		/* The index is still clobbered from do_diff_cache() */

diff --git a/tree-diff.c b/tree-diff.c
index aa0a100..a2fdf86 100644
--- a/tree-diff.c
+++ b/tree-diff.c

@@ -326,6 +326,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 		die("unable to set up diff options to follow renames");
 	diff_tree(t1, t2, base, &diff_opts);
 	diffcore_std(&diff_opts);
+	diff_tree_release_paths(&diff_opts);
 
 	/* Go through the new set of filepairing, and see if we find a more interesting one */
 	for (i = 0; i < q->nr; i++) {

@@ -418,7 +419,8 @@ static int count_paths(const char **paths)
 
 void diff_tree_release_paths(struct diff_options *opt)
 {
-	free(opt->pathlens);
+	if (opt->pathlens)
+		free(opt->pathlens);
 }
 
 void diff_tree_setup_paths(const char **p, struct diff_options *opt)

-- 
1.5.3.7

[PATCH 1/2 v2] Fix XML parser leaks in http-push

From: Mike Hommey <hidden>
Date: 2016-06-15 22:43:58

XML_Parser were never freed. While at it, move the parser initialization to
right before it is needed.

Signed-off-by: Mike Hommey <redacted>
---

 This one is the same, but against pu, where Junio fixed my strbuf patch in
 a different way than I did.

 http-push.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/http-push.c b/http-push.c
index a76a9e5..c7586b3 100644
--- a/http-push.c
+++ b/http-push.c

@@ -1189,8 +1189,6 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	char *ep;
 	char timeout_header[25];
 	struct remote_lock *lock = NULL;
-	XML_Parser parser = XML_ParserCreate(NULL);
-	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
 	struct xml_ctx ctx;

@@ -1250,6 +1248,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result == CURLE_OK) {
+			XML_Parser parser = XML_ParserCreate(NULL);
+			enum XML_Status result;
 			ctx.name = xcalloc(10, 1);
 			ctx.len = 0;
 			ctx.cdata = NULL;

@@ -1268,6 +1268,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 						XML_GetErrorCode(parser)));
 				lock->timeout = -1;
 			}
+			XML_ParserFree(parser);
 		}
 	} else {
 		fprintf(stderr, "Unable to start LOCK request\n");

@@ -1428,8 +1429,6 @@ static void remote_ls(const char *path, int flags,
 	struct slot_results results;
 	struct strbuf in_buffer = STRBUF_INIT;
 	struct buffer out_buffer = { STRBUF_INIT, 0 };
-	XML_Parser parser = XML_ParserCreate(NULL);
-	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
 	struct xml_ctx ctx;
 	struct remote_ls_ctx ls;

@@ -1463,6 +1462,8 @@ static void remote_ls(const char *path, int flags,
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result == CURLE_OK) {
+			XML_Parser parser = XML_ParserCreate(NULL);
+			enum XML_Status result;
 			ctx.name = xcalloc(10, 1);
 			ctx.len = 0;
 			ctx.cdata = NULL;

@@ -1481,6 +1482,7 @@ static void remote_ls(const char *path, int flags,
 					XML_ErrorString(
 						XML_GetErrorCode(parser)));
 			}
+			XML_ParserFree(parser);
 		}
 	} else {
 		fprintf(stderr, "Unable to start PROPFIND request\n");

@@ -1512,8 +1514,6 @@ static int locking_available(void)
 	struct slot_results results;
 	struct strbuf in_buffer = STRBUF_INIT;
 	struct buffer out_buffer = { STRBUF_INIT, 0 };
-	XML_Parser parser = XML_ParserCreate(NULL);
-	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
 	struct xml_ctx ctx;
 	int lock_flags = 0;

@@ -1538,6 +1538,8 @@ static int locking_available(void)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result == CURLE_OK) {
+			XML_Parser parser = XML_ParserCreate(NULL);
+			enum XML_Status result;
 			ctx.name = xcalloc(10, 1);
 			ctx.len = 0;
 			ctx.cdata = NULL;

@@ -1556,6 +1558,7 @@ static int locking_available(void)
 						XML_GetErrorCode(parser)));
 				lock_flags = 0;
 			}
+			XML_ParserFree(parser);
 		}
 	} else {
 		fprintf(stderr, "Unable to start PROPFIND request\n");

-- 
1.5.3.7.1164.ga23bb-dirty

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help