{"thread":{"id":"1923","subject":"[PATCH 6/6] git-local-fetch: Avoid confusing error messages on packed repositories","startedAt":"2005-09-23T12:27:54Z","lastAt":"2005-09-23T12:28:38Z","messageCount":7,"participants":["Sergey Vlasov"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"9178","messageId":"20050923122754.GB2197@master.mivlgu.local","threadId":"1923","inReplyTo":null,"subject":"[PATCH 0/6] More fetch fixes","fromName":"Sergey Vlasov","fromEmail":"vsu@altlinux.ru","sentAt":"2005-09-23T12:27:54Z","receivedAt":"2005-09-23T12:27:54Z","isPatch":true,"sender":{"key":"vsu@altlinux.ru","avatar":"https://avatars.githubusercontent.com/u/616082?v=4"},"body":"Hello!\n\nHere are some more patches for the fetch.c common code:\n\n1) Disable building of object ref lists (as in git-rev-list)\n2) Fix leak of tree entry names\n\nAnd then some fixes for git-local-fetch:\n\n3) Fix missing closedir() and check for success of opendir()\n4) Avoid close(-1) (not exactly a bug, but Valgrind does not like it)\n5) Fix \"git-local-fetch -s\" not working with packed repositories\n6) Avoid confusing \"cannot open\" error messages before fetching a pack\n\n-- \nSergey Vlasov\n"},{"id":"9177","messageId":"20050923122813.27F56E010FC@center4.mivlgu.local","threadId":"1923","inReplyTo":"20050923122754.GB2197@master.mivlgu.local","subject":"[PATCH 1/6] fetch.c: Do not build object ref lists","fromName":"Sergey Vlasov","fromEmail":"vsu@altlinux.ru","sentAt":"2005-09-23T12:28:13Z","receivedAt":"2005-09-23T12:28:13Z","isPatch":true,"sender":{"key":"vsu@altlinux.ru","avatar":"https://avatars.githubusercontent.com/u/616082?v=4"},"body":"The fetch code does not need object ref lists; by disabling them we\ncan save some time and memory.\n\nSigned-off-by: Sergey Vlasov \n\n\n---\n\n fetch.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ncf77c02b386647f312a86ffc342b010ae55725cd\ndiff --git a/fetch.c b/fetch.c\n--- a/fetch.c\n+++ b/fetch.c\n@@ -206,6 +206,7 @@ int pull(char *target)\n \tint fd = -1;\n \n \tsave_commit_buffer = 0;\n+\ttrack_object_refs = 0;\n \tif (write_ref && current_ref) {\n \t\tfd = lock_ref_sha1(write_ref, current_ref);\n \t\tif (fd < 0)\n"},{"id":"9175","messageId":"20050923122818.39DFFE01C96@center4.mivlgu.local","threadId":"1923","inReplyTo":"20050923122754.GB2197@master.mivlgu.local","subject":"[PATCH 2/6] fetch.c: Plug memory leak in process_tree()","fromName":"Sergey Vlasov","fromEmail":"vsu@altlinux.ru","sentAt":"2005-09-23T12:28:18Z","receivedAt":"2005-09-23T12:28:18Z","isPatch":true,"sender":{"key":"vsu@altlinux.ru","avatar":"https://avatars.githubusercontent.com/u/616082?v=4"},"body":"When freeing a tree entry, must free its name too.\n\nSigned-off-by: Sergey Vlasov \n\n\n---\n\n fetch.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\n33e5d08bcd7c35725f704cb70ffcc7257df61bcb\ndiff --git a/fetch.c b/fetch.c\n--- a/fetch.c\n+++ b/fetch.c\n@@ -48,6 +48,7 @@ static int process_tree(struct tree *tre\n \t\tstruct tree_entry_list *next = entry->next;\n \t\tif (process(entry->item.any))\n \t\t\treturn -1;\n+\t\tfree(entry->name);\n \t\tfree(entry);\n \t\tentry = next;\n \t}\n"},{"id":"9179","messageId":"20050923122823.3D01BE010FC@center4.mivlgu.local","threadId":"1923","inReplyTo":"20050923122754.GB2197@master.mivlgu.local","subject":"[PATCH 3/6] git-local-fetch: Fix error checking and leak in setup_indices()","fromName":"Sergey Vlasov","fromEmail":"vsu@altlinux.ru","sentAt":"2005-09-23T12:28:23Z","receivedAt":"2005-09-23T12:28:23Z","isPatch":true,"sender":{"key":"vsu@altlinux.ru","avatar":"https://avatars.githubusercontent.com/u/616082?v=4"},"body":"setup_indices() did not check the return value of opendir(), and\ndid not have a corresponding closedir() call.\n\nSigned-off-by: Sergey Vlasov \n\n\n---\n\n local-fetch.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\nb2c9aab8c8186900cc4610e144513d45fc5b4865\ndiff --git a/local-fetch.c b/local-fetch.c\n--- a/local-fetch.c\n+++ b/local-fetch.c\n@@ -38,6 +38,8 @@ static int setup_indices(void)\n \tunsigned char sha1[20];\n \tsprintf(filename, \"%s/objects/pack/\", path);\n \tdir = opendir(filename);\n+\tif (!dir)\n+\t\treturn -1;\n \twhile ((de = readdir(dir)) != NULL) {\n \t\tint namelen = strlen(de->d_name);\n \t\tif (namelen != 50 || \n@@ -46,6 +48,7 @@ static int setup_indices(void)\n \t\tget_sha1_hex(de->d_name + 5, sha1);\n \t\tsetup_index(sha1);\n \t}\n+\tclosedir(dir);\n \treturn 0;\n }\n \n"},{"id":"9176","messageId":"20050923122828.43796E010FC@center4.mivlgu.local","threadId":"1923","inReplyTo":"20050923122754.GB2197@master.mivlgu.local","subject":"[PATCH 4/6] git-local-fetch: Avoid calling close(-1)","fromName":"Sergey Vlasov","fromEmail":"vsu@altlinux.ru","sentAt":"2005-09-23T12:28:28Z","receivedAt":"2005-09-23T12:28:28Z","isPatch":true,"sender":{"key":"vsu@altlinux.ru","avatar":"https://avatars.githubusercontent.com/u/616082?v=4"},"body":"After open() failure, copy_file() called close(ifd) with ifd == -1\n(harmless, but causes Valgrind noise). The same thing was possible\nfor the destination file descriptor.\n\nSigned-off-by: Sergey Vlasov \n\n\n---\n\n local-fetch.c | 6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\nbb38750b6016dcd8dc71b2cd0a3bdef035f6508d\ndiff --git a/local-fetch.c b/local-fetch.c\n--- a/local-fetch.c\n+++ b/local-fetch.c\n@@ -75,7 +75,8 @@ static int copy_file(const char *source,\n \t\tvoid *map;\n \t\tifd = open(source, O_RDONLY);\n \t\tif (ifd < 0 || fstat(ifd, &st) < 0) {\n-\t\t\tclose(ifd);\n+\t\t\tif (ifd >= 0)\n+\t\t\t\tclose(ifd);\n \t\t\tfprintf(stderr, \"cannot open %s\\n\", source);\n \t\t\treturn -1;\n \t\t}\n@@ -89,7 +90,8 @@ static int copy_file(const char *source,\n \t\tstatus = ((ofd < 0) ||\n \t\t\t (write(ofd, map, st.st_size) != st.st_size));\n \t\tmunmap(map, st.st_size);\n-\t\tclose(ofd);\n+\t\tif (ofd >= 0)\n+\t\t\tclose(ofd);\n \t\tif (status)\n \t\t\tfprintf(stderr, \"cannot write %s\\n\", dest);\n \t\telse\n"},{"id":"9180","messageId":"20050923122833.49AD2E010FC@center4.mivlgu.local","threadId":"1923","inReplyTo":"20050923122754.GB2197@master.mivlgu.local","subject":"[PATCH 5/6] Fix \"git-local-fetch -s\" with packed source repository","fromName":"Sergey Vlasov","fromEmail":"vsu@altlinux.ru","sentAt":"2005-09-23T12:28:33Z","receivedAt":"2005-09-23T12:28:33Z","isPatch":true,"sender":{"key":"vsu@altlinux.ru","avatar":"https://avatars.githubusercontent.com/u/616082?v=4"},"body":"\"git-local-fetch -s\" did not work with a packed repository, because\nsymlink() happily created a link to a non-existing object file,\ntherefore fetch_file() always returned success, and fetch_pack() was\nnot called. Fixed by calling stat() before symlink() to ensure the\nfile really exists.\n\nSigned-off-by: Sergey Vlasov \n\n\n---\n\n local-fetch.c | 14 +++++++++++---\n 1 files changed, 11 insertions(+), 3 deletions(-)\n\n40d9d7c0c86ee790a330f2fb3e1355bde4e9d434\ndiff --git a/local-fetch.c b/local-fetch.c\n--- a/local-fetch.c\n+++ b/local-fetch.c\n@@ -65,9 +65,17 @@ static int copy_file(const char *source,\n \t\t\treturn -1;\n \t\t}\n \t}\n-\tif (use_symlink && !symlink(source, dest)) {\n-\t\tpull_say(\"symlink %s\\n\", hex);\n-\t\treturn 0;\n+\tif (use_symlink) {\n+\t\tstruct stat st;\n+\t\tif (stat(source, &st)) {\n+\t\t\tfprintf(stderr, \"cannot stat %s: %s\\n\", source,\n+\t\t\t\tstrerror(errno));\n+\t\t\treturn -1;\n+\t\t}\n+\t\tif (!symlink(source, dest)) {\n+\t\t\tpull_say(\"symlink %s\\n\", hex);\n+\t\t\treturn 0;\n+\t\t}\n \t}\n \tif (use_filecopy) {\n \t\tint ifd, ofd, status;\n"},{"id":"9174","messageId":"20050923122838.5107AE010FC@center4.mivlgu.local","threadId":"1923","inReplyTo":"20050923122754.GB2197@master.mivlgu.local","subject":"[PATCH 6/6] git-local-fetch: Avoid confusing error messages on packed repositories","fromName":"Sergey Vlasov","fromEmail":"vsu@altlinux.ru","sentAt":"2005-09-23T12:28:38Z","receivedAt":"2005-09-23T12:28:38Z","isPatch":true,"sender":{"key":"vsu@altlinux.ru","avatar":"https://avatars.githubusercontent.com/u/616082?v=4"},"body":"If the source repository was packed, and git-local-fetch needed to\nfetch a pack file, it spewed a misleading error message about not\nbeing able to find the unpacked object. Fixed by adding the\nwarn_if_not_exists argument to copy_file(), which controls printing\nof error messages in case the source file does not exist.\n\nSigned-off-by: Sergey Vlasov \n\n\n---\n\n local-fetch.c | 17 ++++++++++++-----\n 1 files changed, 12 insertions(+), 5 deletions(-)\n\n3417063eaaf7a9293621a72c8b5df4653daf04ea\ndiff --git a/local-fetch.c b/local-fetch.c\n--- a/local-fetch.c\n+++ b/local-fetch.c\n@@ -52,7 +52,8 @@ static int setup_indices(void)\n \treturn 0;\n }\n \n-static int copy_file(const char *source, const char *dest, const char *hex)\n+static int copy_file(const char *source, const char *dest, const char *hex,\n+\t\t int warn_if_not_exists)\n {\n \tif (use_link) {\n \t\tif (!link(source, dest)) {\n@@ -61,13 +62,16 @@ static int copy_file(const char *source,\n \t\t}\n \t\t/* If we got ENOENT there is no point continuing. */\n \t\tif (errno == ENOENT) {\n-\t\t\tfprintf(stderr, \"does not exist %s\\n\", source);\n+\t\t\tif (warn_if_not_exists)\n+\t\t\t\tfprintf(stderr, \"does not exist %s\\n\", source);\n \t\t\treturn -1;\n \t\t}\n \t}\n \tif (use_symlink) {\n \t\tstruct stat st;\n \t\tif (stat(source, &st)) {\n+\t\t\tif (!warn_if_not_exists && errno == ENOENT)\n+\t\t\t\treturn -1;\n \t\t\tfprintf(stderr, \"cannot stat %s: %s\\n\", source,\n \t\t\t\tstrerror(errno));\n \t\t\treturn -1;\n@@ -83,8 +87,11 @@ static int copy_file(const char *source,\n \t\tvoid *map;\n \t\tifd = open(source, O_RDONLY);\n \t\tif (ifd < 0 || fstat(ifd, &st) < 0) {\n+\t\t\tint err = errno;\n \t\t\tif (ifd >= 0)\n \t\t\t\tclose(ifd);\n+\t\t\tif (!warn_if_not_exists && err == ENOENT)\n+\t\t\t\treturn -1;\n \t\t\tfprintf(stderr, \"cannot open %s\\n\", source);\n \t\t\treturn -1;\n \t\t}\n@@ -129,11 +136,11 @@ static int fetch_pack(const unsigned cha\n \tsprintf(filename, \"%s/objects/pack/pack-%s.pack\", \n \t\tpath, sha1_to_hex(target->sha1));\n \tcopy_file(filename, sha1_pack_name(target->sha1),\n-\t\t sha1_to_hex(target->sha1));\n+\t\t sha1_to_hex(target->sha1), 1);\n \tsprintf(filename, \"%s/objects/pack/pack-%s.idx\", \n \t\tpath, sha1_to_hex(target->sha1));\n \tcopy_file(filename, sha1_pack_index_name(target->sha1),\n-\t\t sha1_to_hex(target->sha1));\n+\t\t sha1_to_hex(target->sha1), 1);\n \tinstall_packed_git(target);\n \treturn 0;\n }\n@@ -154,7 +161,7 @@ static int fetch_file(const unsigned cha\n \tfilename[object_name_start+1] = hex[1];\n \tfilename[object_name_start+2] = '/';\n \tstrcpy(filename + object_name_start + 3, hex + 2);\n-\treturn copy_file(filename, dest_filename, hex);\n+\treturn copy_file(filename, dest_filename, hex, 0);\n }\n \n int fetch(unsigned char *sha1)\n"}]}