[PATCH 0/6] More fetch fixes

[PATCH 0/6] More fetch fixes

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

Hello!

Here are some more patches for the fetch.c common code:

1) Disable building of object ref lists (as in git-rev-list)
2) Fix leak of tree entry names

And then some fixes for git-local-fetch:

3) Fix missing closedir() and check for success of opendir()
4) Avoid close(-1) (not exactly a bug, but Valgrind does not like it)
5) Fix "git-local-fetch -s" not working with packed repositories
6) Avoid confusing "cannot open" error messages before fetching a pack

-- 
Sergey Vlasov

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 1/6] fetch.c: Do not build object ref lists

[Sergey Vlasov]

The fetch code does not need object ref lists; by disabling them we
can save some time and memory.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>


---

 fetch.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

cf77c02b386647f312a86ffc342b010ae55725cd
diff --git a/fetch.c b/fetch.c
--- a/fetch.c
+++ b/fetch.c
@@ -206,6 +206,7 @@ int pull(char *target)
 	int fd = -1;
 
 	save_commit_buffer = 0;
+	track_object_refs = 0;
 	if (write_ref && current_ref) {
 		fd = lock_ref_sha1(write_ref, current_ref);
 		if (fd < 0)

[PATCH 2/6] fetch.c: Plug memory leak in process_tree()

[Sergey Vlasov]

When freeing a tree entry, must free its name too.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>


---

 fetch.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

33e5d08bcd7c35725f704cb70ffcc7257df61bcb
diff --git a/fetch.c b/fetch.c
--- a/fetch.c
+++ b/fetch.c
@@ -48,6 +48,7 @@ static int process_tree(struct tree *tre
 		struct tree_entry_list *next = entry->next;
 		if (process(entry->item.any))
 			return -1;
+		free(entry->name);
 		free(entry);
 		entry = next;
 	}

[PATCH 3/6] git-local-fetch: Fix error checking and leak in setup_indices()

[Sergey Vlasov]

setup_indices() did not check the return value of opendir(), and
did not have a corresponding closedir() call.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>


---

 local-fetch.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

b2c9aab8c8186900cc4610e144513d45fc5b4865
diff --git a/local-fetch.c b/local-fetch.c
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -38,6 +38,8 @@ static int setup_indices(void)
 	unsigned char sha1[20];
 	sprintf(filename, "%s/objects/pack/", path);
 	dir = opendir(filename);
+	if (!dir)
+		return -1;
 	while ((de = readdir(dir)) != NULL) {
 		int namelen = strlen(de->d_name);
 		if (namelen != 50 || 
@@ -46,6 +48,7 @@ static int setup_indices(void)
 		get_sha1_hex(de->d_name + 5, sha1);
 		setup_index(sha1);
 	}
+	closedir(dir);
 	return 0;
 }
 

[PATCH 4/6] git-local-fetch: Avoid calling close(-1)

[Sergey Vlasov]

After open() failure, copy_file() called close(ifd) with ifd == -1
(harmless, but causes Valgrind noise).  The same thing was possible
for the destination file descriptor.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>


---

 local-fetch.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

bb38750b6016dcd8dc71b2cd0a3bdef035f6508d
diff --git a/local-fetch.c b/local-fetch.c
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -75,7 +75,8 @@ static int copy_file(const char *source,
 		void *map;
 		ifd = open(source, O_RDONLY);
 		if (ifd < 0 || fstat(ifd, &st) < 0) {
-			close(ifd);
+			if (ifd >= 0)
+				close(ifd);
 			fprintf(stderr, "cannot open %s\n", source);
 			return -1;
 		}
@@ -89,7 +90,8 @@ static int copy_file(const char *source,
 		status = ((ofd < 0) ||
 			  (write(ofd, map, st.st_size) != st.st_size));
 		munmap(map, st.st_size);
-		close(ofd);
+		if (ofd >= 0)
+			close(ofd);
 		if (status)
 			fprintf(stderr, "cannot write %s\n", dest);
 		else

[PATCH 5/6] Fix "git-local-fetch -s" with packed source repository

[Sergey Vlasov]

"git-local-fetch -s" did not work with a packed repository, because
symlink() happily created a link to a non-existing object file,
therefore fetch_file() always returned success, and fetch_pack() was
not called.  Fixed by calling stat() before symlink() to ensure the
file really exists.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>


---

 local-fetch.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

40d9d7c0c86ee790a330f2fb3e1355bde4e9d434
diff --git a/local-fetch.c b/local-fetch.c
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -65,9 +65,17 @@ static int copy_file(const char *source,
 			return -1;
 		}
 	}
-	if (use_symlink && !symlink(source, dest)) {
-		pull_say("symlink %s\n", hex);
-		return 0;
+	if (use_symlink) {
+		struct stat st;
+		if (stat(source, &st)) {
+			fprintf(stderr, "cannot stat %s: %s\n", source,
+				strerror(errno));
+			return -1;
+		}
+		if (!symlink(source, dest)) {
+			pull_say("symlink %s\n", hex);
+			return 0;
+		}
 	}
 	if (use_filecopy) {
 		int ifd, ofd, status;

[PATCH 6/6] git-local-fetch: Avoid confusing error messages on packed repositories

[Sergey Vlasov]

If the source repository was packed, and git-local-fetch needed to
fetch a pack file, it spewed a misleading error message about not
being able to find the unpacked object.  Fixed by adding the
warn_if_not_exists argument to copy_file(), which controls printing
of error messages in case the source file does not exist.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>


---

 local-fetch.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

3417063eaaf7a9293621a72c8b5df4653daf04ea
diff --git a/local-fetch.c b/local-fetch.c
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -52,7 +52,8 @@ static int setup_indices(void)
 	return 0;
 }
 
-static int copy_file(const char *source, const char *dest, const char *hex)
+static int copy_file(const char *source, const char *dest, const char *hex,
+		     int warn_if_not_exists)
 {
 	if (use_link) {
 		if (!link(source, dest)) {
@@ -61,13 +62,16 @@ static int copy_file(const char *source,
 		}
 		/* If we got ENOENT there is no point continuing. */
 		if (errno == ENOENT) {
-			fprintf(stderr, "does not exist %s\n", source);
+			if (warn_if_not_exists)
+				fprintf(stderr, "does not exist %s\n", source);
 			return -1;
 		}
 	}
 	if (use_symlink) {
 		struct stat st;
 		if (stat(source, &st)) {
+			if (!warn_if_not_exists && errno == ENOENT)
+				return -1;
 			fprintf(stderr, "cannot stat %s: %s\n", source,
 				strerror(errno));
 			return -1;
@@ -83,8 +87,11 @@ static int copy_file(const char *source,
 		void *map;
 		ifd = open(source, O_RDONLY);
 		if (ifd < 0 || fstat(ifd, &st) < 0) {
+			int err = errno;
 			if (ifd >= 0)
 				close(ifd);
+			if (!warn_if_not_exists && err == ENOENT)
+				return -1;
 			fprintf(stderr, "cannot open %s\n", source);
 			return -1;
 		}
@@ -129,11 +136,11 @@ static int fetch_pack(const unsigned cha
 	sprintf(filename, "%s/objects/pack/pack-%s.pack", 
 		path, sha1_to_hex(target->sha1));
 	copy_file(filename, sha1_pack_name(target->sha1),
-		  sha1_to_hex(target->sha1));
+		  sha1_to_hex(target->sha1), 1);
 	sprintf(filename, "%s/objects/pack/pack-%s.idx", 
 		path, sha1_to_hex(target->sha1));
 	copy_file(filename, sha1_pack_index_name(target->sha1),
-		  sha1_to_hex(target->sha1));
+		  sha1_to_hex(target->sha1), 1);
 	install_packed_git(target);
 	return 0;
 }
@@ -154,7 +161,7 @@ static int fetch_file(const unsigned cha
 	filename[object_name_start+1] = hex[1];
 	filename[object_name_start+2] = '/';
 	strcpy(filename + object_name_start + 3, hex + 2);
-	return copy_file(filename, dest_filename, hex);
+	return copy_file(filename, dest_filename, hex, 0);
 }
 
 int fetch(unsigned char *sha1)