All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Michael Haggerty" <mhagger@alum.mit.edu>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH 3/3] cmd_fetch_pack(): fix constness problem and memory leak
Date: Wed,  2 May 2012 21:38:10 +0700	[thread overview]
Message-ID: <1335969490-9181-4-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <1335969490-9181-1-git-send-email-pclouds@gmail.com>

From: Michael Haggerty <mhagger@alum.mit.edu>

The old code cast away the constness of the strings passed to the
function in argument argv[], which could result in their being
modified by filter_refs().  Moreover, if refs were passed via stdin,
then the memory allocated for them was never freed (though, of course,
this function is only called once so it is not a real problem).

Fix both errors by copying *all* reference names into our own array
and always freeing the array at the end of the function.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fetch-pack.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 65ac111..beabdc2 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -902,10 +902,11 @@ static void fetch_pack_setup(void)
 
 int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 {
-	int i, ret, nr_heads;
+	int i, ret;
 	struct ref *ref = NULL;
 	const char *dest = NULL;
-	char **heads;
+	int alloc_heads = 0, nr_heads = 0;
+	char **heads = NULL;
 	int fd[2];
 	char *pack_lockfile = NULL;
 	char **pack_lockfile_ptr = NULL;
@@ -934,8 +935,6 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 
 	packet_trace_identity("fetch-pack");
 
-	nr_heads = 0;
-	heads = NULL;
 	argc = parse_options(argc, argv, prefix, opts, fetch_pack_usage, 0);
 	args.no_progress = !progress;
 	if (args.keep_pack > 1)
@@ -945,19 +944,19 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 	dest = argv[0];
 	if (!argc || !dest)
 		usage_with_options(fetch_pack_usage, opts);
-	heads = (char **)(argv + 1);
-	nr_heads = argc - 1;
+
+	/*
+	 * Copy refs from cmdline to new growable list, then append
+	 * any refs from the standard input.
+	 */
+	ALLOC_GROW(heads, argc - 1, alloc_heads);
+	for (i = 1; i < argc; i++)
+		heads[nr_heads++] = xstrdup(argv[i]);
 
 	if (args.stdin_refs) {
-		/*
-		 * Copy refs from cmdline to new growable list, then
-		 * append the refs from the standard input.
-		 */
-		int alloc_heads = nr_heads;
-		int size = nr_heads * sizeof(*heads);
-		heads = memcpy(xmalloc(size), heads, size);
 		if (args.stateless_rpc) {
-			/* in stateless RPC mode we use pkt-line to read
+			/*
+			 * in stateless RPC mode we use pkt-line to read
 			 * from stdin, until we get a flush packet
 			 */
 			static char line[1000];
@@ -1023,6 +1022,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 		ref = ref->next;
 	}
 
+	for (i = 0; i < nr_heads; ++i)
+		free(heads[i]);
+	free(heads);
+
 	return ret;
 }
 
-- 
1.7.8.36.g69ee2

  parent reply	other threads:[~2012-05-02 14:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-02 10:40 [PATCH 0/2] Fix some constness errors in fetch-pack mhagger
2012-05-02 10:40 ` [PATCH 1/2] cmd_fetch_pack(): declare dest to be const mhagger
2012-05-02 10:40 ` [PATCH 2/2] cmd_fetch_pack(): fix constness problem and memory leak mhagger
2012-05-02 11:14   ` Nguyen Thai Ngoc Duy
2012-05-02 13:35     ` Michael Haggerty
2012-05-02 14:38       ` [PATCH 0/3] Fix some constness errors in fetch-pack and parseopt conversion Nguyễn Thái Ngọc Duy
2012-05-02 14:38         ` [PATCH 1/3] cmd_fetch_pack(): declare dest to be const Nguyễn Thái Ngọc Duy
2012-05-02 14:38         ` [PATCH 2/3] fetch-pack: use parse_options() Nguyễn Thái Ngọc Duy
2012-05-02 14:38         ` Nguyễn Thái Ngọc Duy [this message]
2012-05-02 17:14     ` [PATCH 2/2] cmd_fetch_pack(): fix constness problem and memory leak Junio C Hamano
2012-05-21  1:47   ` Junio C Hamano
2012-05-21  8:13     ` Michael Haggerty
2012-05-19 14:05 ` [PATCH 0/2] Fix some constness errors in fetch-pack Michael Haggerty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1335969490-9181-4-git-send-email-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.