[PATCH 3/3] http-push: send out fetch requests on queue

Previously, requests for remote files were simply added to the queue
(pointed to by request_queue_head) and no transfer actually took
place[1], even though code that followed may rely on these remote files
to be present (eg. the setup_revisions invocation).

The code that sends out the requests on the request queue is refactored
into the method run_request_queue.

After the get_dav_remote_heads invocation (ie. after fetch requests are
added to the queue), the requests on the queue are sent out through an
invocation to run_request_queue.

This invocation to run_request_queue entails adding a fill function
before pushing checks take place, which may lead to accidental,
unwanted pushes previously.

The flag is_running_queue is introduced to prevent this from occurring.
fill_active_slot is made to check the flag is_running_queue before
the sending of the requests proceeds.

Signed-off-by: Tay Ray Chuan <redacted>
---
 http-push.c          |   37 ++++++++++++++++++++++++++-----------
 t/t5540-http-push.sh |    4 ++--
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/http-push.c b/http-push.c
index 5138224..1ba7987 100644
--- a/http-push.c
+++ b/http-push.c

@@ -843,11 +843,12 @@ static void finish_request(struct transfer_request *request)
 }

 #ifdef USE_CURL_MULTI
+static int is_running_queue;
 static int fill_active_slot(void *unused)
 {
 	struct transfer_request *request;

-	if (aborted)
+	if (aborted || !is_running_queue)
 		return 0;

 	for (request = request_queue_head; request; request = request->next) {

@@ -2167,6 +2168,25 @@ static int delete_remote_branch(char *pattern, int force)
 	return 0;
 }

+void run_request_queue()
+{
+#ifdef USE_CURL_MULTI
+	is_running_queue = 1;
+	fill_active_slots();
+	add_fill_function(NULL, fill_active_slot);
+#endif
+	do {
+		finish_all_active_slots();
+#ifdef USE_CURL_MULTI
+		fill_active_slots();
+#endif
+	} while (request_queue_head && !aborted);
+
+#ifdef USE_CURL_MULTI
+	is_running_queue = 0;
+#endif
+}
+
 int main(int argc, char **argv)
 {
 	struct transfer_request *request;

@@ -2271,6 +2291,8 @@ int main(int argc, char **argv)
 		repo->url = rewritten_url;
 	}

+	is_running_queue = 0;
+
 	/* Verify DAV compliance/lock support */
 	if (!locking_available()) {
 		rc = 1;

@@ -2300,6 +2322,7 @@ int main(int argc, char **argv)
 	local_refs = get_local_heads();
 	fprintf(stderr, "Fetching remote heads...\n");
 	get_dav_remote_heads();
+	run_request_queue();

 	/* Remove a remote branch if -d or -D was specified */
 	if (delete_branch) {

@@ -2429,16 +2452,8 @@ int main(int argc, char **argv)
 		if (objects_to_send)
 			fprintf(stderr, "    sending %d objects\n",
 				objects_to_send);
-#ifdef USE_CURL_MULTI
-		fill_active_slots();
-		add_fill_function(NULL, fill_active_slot);
-#endif
-		do {
-			finish_all_active_slots();
-#ifdef USE_CURL_MULTI
-			fill_active_slots();
-#endif
-		} while (request_queue_head && !aborted);
+
+		run_request_queue();

 		/* Update the remote branch if all went well */
 		if (aborted || !update_remote(ref->new_sha1, ref_lock))

diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index ad0f14b..f4a2cf6 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh

@@ -67,7 +67,7 @@ test_expect_success ' push to remote repository with unpacked refs' '
 	 test $HEAD = $(git rev-parse --verify HEAD))
 '

-test_expect_failure 'http-push fetches unpacked objects' '
+test_expect_success 'http-push fetches unpacked objects' '
 	cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
 		"$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_unpacked.git &&

@@ -83,7 +83,7 @@ test_expect_failure 'http-push fetches unpacked objects' '
 	 git push -f -v $HTTPD_URL/test_repo_unpacked.git master)
 '

-test_expect_failure 'http-push fetches packed objects' '
+test_expect_success 'http-push fetches packed objects' '
 	cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
 		"$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_packed.git &&

-- 
1.6.3.rc0

Hi,

On Sat, Apr 25, 2009 at 12:35:57AM +0800, Tay Ray Chuan wrote:
[...]

The "http-push fetches packed objects" test of t5540 on pu either segfaults
or hangs indefinitely on my machine [1]. The bug is revealed by the added
line above in commit 0d71b15 (http-push: send out fetch requests on queue).

I have been trying to debug this, but I'm at a loss. Below are the backtrace
and an excerpt of the valgrind output with 0d71b15. See
http://gist.github.com/120453 for the full output. Note that the
segfault/hang does not happen if valgrind is used.

Can anybody reproduce this? Set GIT_TEST_HTTPD to enable this test.

Clemens

[1] Debian amd64 stable (mostly), libcurl version 17.18.2
---

(gdb) r
Starting program: /home/drizzd/fridge/linux/src/git/git-http-push --force
--verbose http://127.0.0.1:5540/test_repo_packed.git master
[Thread debugging using libthread_db enabled]
Getting pack list
Getting index for pack ecba592576b66d9aa805891d0a80a0133abd5f5d
Fetching remote heads...
  refs/
  refs/heads/
  fetch 9d498b0bbc2a25438e2fbd19081948da86028c23 for refs/heads/master
  refs/tags/
Fetching pack ecba592576b66d9aa805891d0a80a0133abd5f5d
 which contains 9d498b0bbc2a25438e2fbd19081948da86028c23
[New Thread 0x7f60404a9700 (LWP 21395)]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f60404a9700 (LWP 21395)]
0x00007f603f49245e in ?? () from /lib/libc.so.6
(gdb) bt
#0  0x00007f603f49245e in ?? () from /lib/libc.so.6
#1  0x00007f603f49104a in ftell () from /lib/libc.so.6
#2  0x0000000000409785 in run_active_slot (slot=0x6bab40) at http.c:536
#3  0x00000000004099f0 in finish_all_active_slots () at http.c:607
#4  0x000000000040f72f in run_request_queue () at http-push.c:2179
#5  0x000000000040fcb4 in main (argc=5, argv=0x7fff485cd748) at
http-push.c:2325


* checking known breakage: 
	cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
		"$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_packed.git &&

	git clone $HTTPD_URL/test_repo_packed.git \
		"$ROOT_PATH"/test_repo_clone_packed &&

	(cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_packed.git &&
	 git --bare repack &&
	 git --bare prune-packed) &&

	# By reset, we force git to retrieve the packed object
	(cd "$ROOT_PATH"/test_repo_clone_packed &&
	 git reset --hard HEAD^ &&
	 git remote rm origin &&
	 git reflog expire --expire=0 --all &&
	 git prune &&
	 valgrind git-http-push --force --verbose $HTTPD_URL/test_repo_packed.git master)

got 9d498b0bbc2a25438e2fbd19081948da86028c23
walk 9d498b0bbc2a25438e2fbd19081948da86028c23
got 50b820aea6d3503362343cdc0e699b760c700b2b
got 0c973ae9bd51902a28466f3850b543fa66a6aaf4
got e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
walk 0c973ae9bd51902a28466f3850b543fa66a6aaf4
got 146e9fd4040abc05619b6c07a3a23cba27a2c722
Initialized empty Git repository in /home/drizzd/fridge/linux/src/git/t/trash directory.t5540-http-push/test_repo_clone_packed/.git/
HEAD is now at 0c973ae initial
==== Memcheck, a memory error detector.
==== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==== Using LibVEX rev 1854, a library for dynamic binary translation.
==== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==== Using valgrind-3.3.1-Debian, a dynamic binary instrumentation framework.
==== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==== For more details, rerun with: -v
==== 
==== Invalid read of size 1
====    at 0x409EBE: xml_entities (http-push.c:192)
====    by 0x40DE29: locking_available (http-push.c:1619)
====    by 0x40FBCF: main (http-push.c:2297)
====  Address 0x80b94e4 is 0 bytes after a block of size 44 alloc'd
====    at 0x4C2260E: malloc (vg_replace_malloc.c:207)
====    by 0x44C4E4: xmalloc (wrapper.c:20)
====    by 0x40FB4D: main (http-push.c:2286)
Getting pack list
Getting index for pack ecba592576b66d9aa805891d0a80a0133abd5f5d
Fetching remote heads...
  refs/
  refs/heads/
  fetch 9d498b0bbc2a25438e2fbd19081948da86028c23 for refs/heads/master
  refs/tags/
Fetching pack ecba592576b66d9aa805891d0a80a0133abd5f5d
 which contains 9d498b0bbc2a25438e2fbd19081948da86028c23
==== 
==== Invalid read of size 2
====    at 0x5ABFFE1: ftell (in /lib/libc-2.7.so)
====    by 0x409784: run_active_slot (http.c:536)
====    by 0x4099EF: finish_all_active_slots (http.c:607)
====    by 0x40F72E: run_request_queue (http-push.c:2179)
====    by 0x40FCB3: main (http-push.c:2325)
====  Address 0x817c688 is 0 bytes inside a block of size 568 free'd
====    at 0x4C2130F: free (vg_replace_malloc.c:323)
====    by 0x5ABEDA0: fclose (in /lib/libc-2.7.so)
====    by 0x40BC64: finish_request (http-push.c:824)
====    by 0x40A0F4: process_response (http-push.c:257)
====    by 0x4099C4: finish_active_slot (http.c:598)
====    by 0x4088E3: process_curl_messages (http.c:101)
====    by 0x409709: step_active_slots (http.c:512)
====    by 0x40975A: run_active_slot (http.c:533)
====    by 0x4099EF: finish_all_active_slots (http.c:607)
====    by 0x40F72E: run_request_queue (http-push.c:2179)
====    by 0x40FCB3: main (http-push.c:2325)
[...]
==== Invalid read of size 4
====    at 0x5AC00AC: ftell (in /lib/libc-2.7.so)
====    by 0x409784: run_active_slot (http.c:536)
====    by 0x4099EF: finish_all_active_slots (http.c:607)
====    by 0x40F72E: run_request_queue (http-push.c:2179)
====    by 0x40FCB3: main (http-push.c:2325)
====  Address 0x817c768 is 224 bytes inside a block of size 568 free'd
====    at 0x4C2130F: free (vg_replace_malloc.c:323)
====    by 0x5ABEDA0: fclose (in /lib/libc-2.7.so)
====    by 0x40BC64: finish_request (http-push.c:824)
====    by 0x40A0F4: process_response (http-push.c:257)
====    by 0x4099C4: finish_active_slot (http.c:598)
====    by 0x4088E3: process_curl_messages (http.c:101)
====    by 0x409709: step_active_slots (http.c:512)
====    by 0x40975A: run_active_slot (http.c:533)
====    by 0x4099EF: finish_all_active_slots (http.c:607)
====    by 0x40F72E: run_request_queue (http-push.c:2179)
====    by 0x40FCB3: main (http-push.c:2325)
updating 'refs/heads/master'
  from 9d498b0bbc2a25438e2fbd19081948da86028c23
  to   0c973ae9bd51902a28466f3850b543fa66a6aaf4
    done
Updating remote server info
==== 
==== ERROR SUMMARY: 46 errors from 42 contexts (suppressed: 8 from 1)
==== malloc/free: in use at exit: 201,408 bytes in 54 blocks.
==== malloc/free: 2,976 allocs, 2,922 frees, 1,512,372 bytes allocated.
==== For counts of detected errors, rerun with: -v
==== searching for pointers to 54 not-freed blocks.
==== checked 648,992 bytes.
==== 
==== LEAK SUMMARY:
====    definitely lost: 10,463 bytes in 26 blocks.
====      possibly lost: 0 bytes in 0 blocks.
====    still reachable: 190,945 bytes in 28 blocks.
====         suppressed: 0 bytes in 0 blocks.
==== Rerun with --leak-check=full to see details of leaked memory.
*   FIXED 6: http-push fetches packed objects

* fixed 1 known breakage(s)
* still have 1 known breakage(s)
* passed all remaining 5 test(s)

Hi,

On Sat, May 30, 2009 at 5:17 PM, Clemens Buchacher [off-list ref] wrote:

The "http-push fetches packed objects" test of t5540 on pu either segfaults
or hangs indefinitely on my machine [1]. The bug is revealed by the added
line above in commit 0d71b15 (http-push: send out fetch requests on queue).

if you don't mind, could you try running this test on the
http-fix-push-fetching branch in git://github.com/rctay/git.git and
see if this still happens? It contains these patches only, applied on
master.

-- 
Cheers,
Ray Chuan

On Sat, May 30, 2009 at 05:31:36PM +0800, Tay Ray Chuan wrote:

On Sat, May 30, 2009 at 5:17 PM, Clemens Buchacher [off-list ref] wrote:

quoted

The "http-push fetches packed objects" test of t5540 on pu either segfaults
or hangs indefinitely on my machine [1]. The bug is revealed by the added
line above in commit 0d71b15 (http-push: send out fetch requests on queue).

if you don't mind, could you try running this test on the
http-fix-push-fetching branch in git://github.com/rctay/git.git and
see if this still happens? It contains these patches only, applied on
master.

Sure. The same thing happens.

Hi,

On Sat, May 30, 2009 at 5:37 PM, Clemens Buchacher [off-list ref] wrote:

Sure. The same thing happens.

curiously, I wasn't able to reproduce on my cygwin setup, but on my ubuntu box.

-- 
Cheers,
Ray Chuan

Set slot->local to NULL after doing a fclose on the FILE* pointer it
points to.

Move NULL assignment to request->slot from
http-push.c::finish_request() to http-push.c::release_request(). This
is safe, since the functions finish_request() invoke will overwrite
request->slot anyway.

Refactor http-push.c::fetch_index(), http-walker.c::fetch_index() and
http-walker.c::fetch_pack() to use labels while cleaning up.

This fixes the issue raised by Clemens Buchacher on 30th May:

  http://www.spinics.net/lists/git/msg104623.html

Signed-off-by: Tay Ray Chuan <redacted>
---

This applies on master.

 http-push.c   |   23 ++++++++++++++---------
 http-walker.c |   36 +++++++++++++++++++++++-------------
 2 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/http-push.c b/http-push.c
index dac2c6e..816824a 100644
--- a/http-push.c
+++ b/http-push.c

@@ -715,6 +715,8 @@ static void release_request(struct transfer_request *request)
 		close(request->local_fileno);
 	if (request->local_stream)
 		fclose(request->local_stream);
+	if (request->slot)
+		request->slot = NULL;
 	free(request->url);
 	free(request);
 }

@@ -727,7 +729,6 @@ static void finish_request(struct transfer_request *request)
 
 	request->curl_result = request->slot->curl_result;
 	request->http_code = request->slot->http_code;
-	request->slot = NULL;
 
 	/* Keep locks active */
 	check_locks();

@@ -823,6 +824,7 @@ static void finish_request(struct transfer_request *request)
 
 			fclose(request->local_stream);
 			request->local_stream = NULL;
+			request->slot->local = NULL;
 			if (!move_temp_to_file(request->tmpfile,
 					       request->filename)) {
 				target = (struct packed_git *)request->userData;

@@ -946,6 +948,7 @@ static int add_send_request(struct object *obj, struct remote_lock *lock)
 
 static int fetch_index(unsigned char *sha1)
 {
+	int ret;
 	char *hex = sha1_to_hex(sha1);
 	char *filename;
 	char *url;

@@ -1022,21 +1025,23 @@ static int fetch_index(unsigned char *sha1)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			free(url);
-			fclose(indexfile);
-			return error("Unable to get pack index %s\n%s", url,
-				     curl_errorstr);
+			ret = error("Unable to get pack index %s\n%s", url,
+				    curl_errorstr);
+			goto cleanup;
 		}
 	} else {
-		free(url);
-		fclose(indexfile);
-		return error("Unable to start request");
+		ret = error("Unable to start request");
+		goto cleanup;
 	}
 
+	ret = move_temp_to_file(tmpfile, filename);
+
+cleanup:
 	free(url);
 	fclose(indexfile);
+	slot->local = NULL;
 
-	return move_temp_to_file(tmpfile, filename);
+	return ret;
 }
 
 static int setup_index(unsigned char *sha1)

diff --git a/http-walker.c b/http-walker.c
index 7321ccc..779947d 100644
--- a/http-walker.c
+++ b/http-walker.c

@@ -364,6 +364,7 @@ static void prefetch(struct walker *walker, unsigned char *sha1)
 
 static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1)
 {
+	int ret;
 	char *hex = sha1_to_hex(sha1);
 	char *filename;
 	char *url;

@@ -417,18 +418,21 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			fclose(indexfile);
-			return error("Unable to get pack index %s\n%s", url,
-				     curl_errorstr);
+			ret = error("Unable to get pack index %s\n%s", url,
+				    curl_errorstr);
+			goto cleanup;
 		}
 	} else {
-		fclose(indexfile);
-		return error("Unable to start request");
+		ret = error("Unable to start request");
+		goto cleanup;
 	}
 
-	fclose(indexfile);
+	ret = move_temp_to_file(tmpfile, filename);
 
-	return move_temp_to_file(tmpfile, filename);
+cleanup:
+	fclose(indexfile);
+	slot->local = NULL;
+	return ret;
 }
 
 static int setup_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1)

@@ -718,7 +722,7 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha
 	FILE *packfile;
 	char *filename;
 	char tmpfile[PATH_MAX];
-	int ret;
+	int ret = 0;
 	long prev_posn = 0;
 	char range[RANGE_HEADER_SIZE];
 	struct curl_slist *range_header = NULL;

@@ -775,17 +779,23 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			fclose(packfile);
-			return error("Unable to get pack file %s\n%s", url,
-				     curl_errorstr);
+			ret = error("Unable to get pack file %s\n%s", url,
+				    curl_errorstr);
+			goto cleanup;
 		}
 	} else {
-		fclose(packfile);
-		return error("Unable to start request");
+		ret = error("Unable to start request");
+		goto cleanup;
 	}
 
 	target->pack_size = ftell(packfile);
+
+cleanup:
 	fclose(packfile);
+	slot->local = NULL;
+
+	if (ret)
+		return ret;
 
 	ret = move_temp_to_file(tmpfile, filename);
 	if (ret)

-- 
1.6.3.1

Set slot->local to NULL after doing a fclose on the FILE* pointer it
points to.

Move NULL assignment to request->slot from
http-push.c::finish_request() to http-push.c::release_request(). This
is safe, since the functions finish_request() invoke will overwrite
request->slot anyway.

Refactor http-push.c::fetch_index(), http-walker.c::fetch_index() and
http-walker.c::fetch_pack() to use labels while cleaning up.

This fixes the issue raised by Clemens Buchacher on 30th May:

  http://www.spinics.net/lists/git/msg104623.html

Signed-off-by: Tay Ray Chuan <redacted>
---

 This applies on master, and is a resend, this time with the subject.

 http-push.c   |   23 ++++++++++++++---------
 http-walker.c |   36 +++++++++++++++++++++++-------------
 2 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/http-push.c b/http-push.c
index dac2c6e..816824a 100644
--- a/http-push.c
+++ b/http-push.c

@@ -715,6 +715,8 @@ static void release_request(struct transfer_request *request)
 		close(request->local_fileno);
 	if (request->local_stream)
 		fclose(request->local_stream);
+	if (request->slot)
+		request->slot = NULL;
 	free(request->url);
 	free(request);
 }

@@ -727,7 +729,6 @@ static void finish_request(struct transfer_request *request)
 
 	request->curl_result = request->slot->curl_result;
 	request->http_code = request->slot->http_code;
-	request->slot = NULL;
 
 	/* Keep locks active */
 	check_locks();

@@ -823,6 +824,7 @@ static void finish_request(struct transfer_request *request)
 
 			fclose(request->local_stream);
 			request->local_stream = NULL;
+			request->slot->local = NULL;
 			if (!move_temp_to_file(request->tmpfile,
 					       request->filename)) {
 				target = (struct packed_git *)request->userData;

@@ -946,6 +948,7 @@ static int add_send_request(struct object *obj, struct remote_lock *lock)
 
 static int fetch_index(unsigned char *sha1)
 {
+	int ret;
 	char *hex = sha1_to_hex(sha1);
 	char *filename;
 	char *url;

@@ -1022,21 +1025,23 @@ static int fetch_index(unsigned char *sha1)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			free(url);
-			fclose(indexfile);
-			return error("Unable to get pack index %s\n%s", url,
-				     curl_errorstr);
+			ret = error("Unable to get pack index %s\n%s", url,
+				    curl_errorstr);
+			goto cleanup;
 		}
 	} else {
-		free(url);
-		fclose(indexfile);
-		return error("Unable to start request");
+		ret = error("Unable to start request");
+		goto cleanup;
 	}
 
+	ret = move_temp_to_file(tmpfile, filename);
+
+cleanup:
 	free(url);
 	fclose(indexfile);
+	slot->local = NULL;
 
-	return move_temp_to_file(tmpfile, filename);
+	return ret;
 }
 
 static int setup_index(unsigned char *sha1)

diff --git a/http-walker.c b/http-walker.c
index 7321ccc..779947d 100644
--- a/http-walker.c
+++ b/http-walker.c

@@ -364,6 +364,7 @@ static void prefetch(struct walker *walker, unsigned char *sha1)
 
 static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1)
 {
+	int ret;
 	char *hex = sha1_to_hex(sha1);
 	char *filename;
 	char *url;

@@ -417,18 +418,21 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			fclose(indexfile);
-			return error("Unable to get pack index %s\n%s", url,
-				     curl_errorstr);
+			ret = error("Unable to get pack index %s\n%s", url,
+				    curl_errorstr);
+			goto cleanup;
 		}
 	} else {
-		fclose(indexfile);
-		return error("Unable to start request");
+		ret = error("Unable to start request");
+		goto cleanup;
 	}
 
-	fclose(indexfile);
+	ret = move_temp_to_file(tmpfile, filename);
 
-	return move_temp_to_file(tmpfile, filename);
+cleanup:
+	fclose(indexfile);
+	slot->local = NULL;
+	return ret;
 }
 
 static int setup_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1)

@@ -718,7 +722,7 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha
 	FILE *packfile;
 	char *filename;
 	char tmpfile[PATH_MAX];
-	int ret;
+	int ret = 0;
 	long prev_posn = 0;
 	char range[RANGE_HEADER_SIZE];
 	struct curl_slist *range_header = NULL;

@@ -775,17 +779,23 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			fclose(packfile);
-			return error("Unable to get pack file %s\n%s", url,
-				     curl_errorstr);
+			ret = error("Unable to get pack file %s\n%s", url,
+				    curl_errorstr);
+			goto cleanup;
 		}
 	} else {
-		fclose(packfile);
-		return error("Unable to start request");
+		ret = error("Unable to start request");
+		goto cleanup;
 	}
 
 	target->pack_size = ftell(packfile);
+
+cleanup:
 	fclose(packfile);
+	slot->local = NULL;
+
+	if (ret)
+		return ret;
 
 	ret = move_temp_to_file(tmpfile, filename);
 	if (ret)

-- 
1.6.3.1

On Sun, May 31, 2009 at 12:09:55AM +0800, Tay Ray Chuan wrote:

Set slot->local to NULL after doing a fclose on the FILE* pointer it
points to.

Thanks. That fixes it.

Clemens