{"thread":{"id":"3438","subject":"Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found","startedAt":"2006-02-22T18:17:58Z","lastAt":"2006-02-23T01:48:16Z","messageCount":8,"participants":["Andrew Vasquez","Junio C Hamano","Carl Worth","Jonas Fonseca"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"16574","messageId":"20060222181758.GH3355@andrew-vasquezs-powerbook-g4-15.local","threadId":"3438","inReplyTo":null,"subject":"Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found","fromName":"Andrew Vasquez","fromEmail":"andrew.vasquez@qlogic.com","sentAt":"2006-02-22T18:17:58Z","receivedAt":"2006-02-22T18:17:58Z","isPatch":true,"sender":{"key":"andrew.vasquez@qlogic.com","avatar":null},"body":"\nCommit:\n\n\tb5b16990f8b074bd0481ced047b8f8bf66eee6dc\n\tPrevent git-upload-pack segfault if object cannot be found\n\nis causing some really annoying noise being sent to stderr on some of\nmy older non-packed repositories:\n\n\t$ git status\n\t# On branch refs/heads/b4\n\tunable to open object pack directory: .git/objects/pack: No such file or directory\n\tnothing to commit\n\n\t$ git-rev-list --pretty=oneline v8.01.04b3..\n\tunable to open object pack directory: .git/objects/pack: No such file or directory\n\ta7401b7109fb2cba7de41a0e30dfe6aa41690ea8 No ZIO.\n\t...\n\n\nHere's the relevant hunk:\n\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 3d11a9b..f08b1d6 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -551,8 +551,10 @@ static void prepare_packed_git_one(char \n>  \tsprintf(path, \"%s/pack\", objdir);\n>  \tlen = strlen(path);\n>  \tdir = opendir(path);\n> -\tif (!dir)\n> +\tif (!dir) {\n> +\t\tfprintf(stderr, \"unable to open object pack directory: %s: %s\\n\", path, strerror(errno));\n>  \t\treturn;\n> +\t}\n>  \tpath[len++] = '/';\n>  \twhile ((de = readdir(dir)) != NULL) {\n>  \t\tint namelen = strlen(de->d_name);\n\n\nCould we drop this fprintf to stderr?\n\nThanks,\nAndrew\n"},{"id":"16575","messageId":"7vpslfntb0.fsf@assigned-by-dhcp.cox.net","threadId":"3438","inReplyTo":"20060222181758.GH3355@andrew-vasquezs-powerbook-g4-15.local","subject":"Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-22T18:26:43Z","receivedAt":"2006-02-22T18:26:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Vasquez <andrew.vasquez@qlogic.com> writes:\n\n> Commit:\n>\n> \tb5b16990f8b074bd0481ced047b8f8bf66eee6dc\n> \tPrevent git-upload-pack segfault if object cannot be found\n>\n> is causing some really annoying noise being sent to stderr on some of\n> my older non-packed repositories:\n>...\n> Could we drop this fprintf to stderr?\n\nThanks for noticing.\n"},{"id":"16578","messageId":"87bqwzs07h.wl%cworth@cworth.org","threadId":"3438","inReplyTo":"20060222181758.GH3355@andrew-vasquezs-powerbook-g4-15.local","subject":"Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found","fromName":"Carl Worth","fromEmail":"cworth@cworth.org","sentAt":"2006-02-22T18:44:02Z","receivedAt":"2006-02-22T18:44:02Z","isPatch":true,"sender":{"key":"cworth@cworth.org","avatar":"https://gravatar.com/avatar/3746dc28cde609bdbd7f939058356e7e2bbd16d21e32274df0725eb3d998bc5b?d=mp&s=160"},"body":"On Wed, 22 Feb 2006 10:17:58 -0800, Andrew Vasquez wrote:\n> Commit:\n> \n> \tb5b16990f8b074bd0481ced047b8f8bf66eee6dc\n> \tPrevent git-upload-pack segfault if object cannot be found\n> \n> is causing some really annoying noise being sent to stderr on some of\n> my older non-packed repositories:\n> \n> \t$ git status\n> \t# On branch refs/heads/b4\n> \tunable to open object pack directory: .git/objects/pack: No such file or directory\n> \tnothing to commit\n\nHmm... I can see that would be really annoying.\n\n> > -\tif (!dir)\n> > +\tif (!dir) {\n> > +\t\tfprintf(stderr, \"unable to open object pack directory: %s: %s\\n\", path, strerror(errno));\n> >  \t\treturn;\n> > +\t}\n> \n> Could we drop this fprintf to stderr?\n\nIn the case in which I hit the original bug, this message is necessary\nto provide information about where the actual problem is. Here is what\nthat scenario looks like with the message:\n\n$ mkdir original;\n$ (cd original; git-init-db; touch foo; git add foo; git commit -m \"original\")\ndefaulting to local storage area\nCommitting initial tree 4d5fcadc293a348e88f777dc0920f11e7d71441c\n$ git clone -l -s original clone\n$ mv original moved\n$ git clone clone again\nunable to open object pack directory: /tmp/original/.git/objects/pack: No such file or directory\nfatal: git-upload-pack: cannot find object 0153d496df669cbe5cecb665dbe6f95b20461917:\nfatal: unexpected EOF\nclone-pack from '/tmp/clone/.git' failed.\n\nHere the \"cannot find object\" message doesn't point to the core\nproblem, but the \"unable to open object pack directory\" does contain\nthe \"/tmp/original\" path of interest.\n\nOne could be a bit more careful about not complaining if <objdir>\nactually does exist, even if <objdir>/pack does not.\n\nOr a workaround would be to just run git-init-db in old repositories\nto create the pack directory:\n\n\t$ rmdir .git/objects/pack/\n\t$ git status\n\tunable to open object pack directory: .git/objects/pack: No such file or directory\n\tnothing to commit\n\t$ git-init-db\n\tdefaulting to local storage area\n\t$ git status\n\tnothing to commit\n\n-Carl\n\n\n"},{"id":"16579","messageId":"7vy803mdnm.fsf@assigned-by-dhcp.cox.net","threadId":"3438","inReplyTo":"87bqwzs07h.wl%cworth@cworth.org","subject":"Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-22T18:50:05Z","receivedAt":"2006-02-22T18:50:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I have this in mind...\n\n---\ndiff --git a/sha1_file.c b/sha1_file.c\nindex f08b1d6..dae77fc 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -552,7 +552,8 @@ static void prepare_packed_git_one(char \n \tlen = strlen(path);\n \tdir = opendir(path);\n \tif (!dir) {\n-\t\tfprintf(stderr, \"unable to open object pack directory: %s: %s\\n\", path, strerror(errno));\n+\t\tif (errno != ENOENT)\n+\t\t\tfprintf(stderr, \"unable to open object pack directory: %s: %s\\n\", path, strerror(errno));\n \t\treturn;\n \t}\n \tpath[len++] = '/';\n"},{"id":"16580","messageId":"7vr75vmcod.fsf@assigned-by-dhcp.cox.net","threadId":"3438","inReplyTo":"87bqwzs07h.wl%cworth@cworth.org","subject":"Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-22T19:11:14Z","receivedAt":"2006-02-22T19:11:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carl Worth <cworth@cworth.org> writes:\n\n> In the case in which I hit the original bug, this message is necessary\n> to provide information about where the actual problem is. Here is what\n> that scenario looks like with the message:\n>\n> $ mkdir original;\n> $ (cd original; git-init-db; touch foo; git add foo; git commit -m \"original\")\n> defaulting to local storage area\n> Committing initial tree 4d5fcadc293a348e88f777dc0920f11e7d71441c\n> $ git clone -l -s original clone\n> $ mv original moved\n> $ git clone clone again\n> unable to open object pack directory: /tmp/original/.git/objects/pack: No such file or directory\n> fatal: git-upload-pack: cannot find object 0153d496df669cbe5cecb665dbe6f95b20461917:\n> fatal: unexpected EOF\n> clone-pack from '/tmp/clone/.git' failed.\n>\n> Here the \"cannot find object\" message doesn't point to the core\n> problem, but the \"unable to open object pack directory\" does contain\n> the \"/tmp/original\" path of interest.\n\nUpdated patch.  The root cause of the problem you had was that\nalternates was dangling, so we catch that.\n\n-- >8 --\ndiff --git a/sha1_file.c b/sha1_file.c\nindex f08b1d6..c08da35 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -247,6 +247,7 @@ static void link_alt_odb_entries(const c\n \t\tfor ( ; cp < ep && *cp != sep; cp++)\n \t\t\t;\n \t\tif (last != cp) {\n+\t\t\tstruct stat st;\n \t\t\tstruct alternate_object_database *alt;\n \t\t\t/* 43 = 40-byte + 2 '/' + terminating NUL */\n \t\t\tint pfxlen = cp - last;\n@@ -269,9 +270,19 @@ static void link_alt_odb_entries(const c\n \t\t\t}\n \t\t\telse\n \t\t\t\tmemcpy(ent->base, last, pfxlen);\n+\n \t\t\tent->name = ent->base + pfxlen + 1;\n-\t\t\tent->base[pfxlen] = ent->base[pfxlen + 3] = '/';\n-\t\t\tent->base[entlen-1] = 0;\n+\t\t\tent->base[pfxlen + 3] = '/';\n+\t\t\tent->base[pfxlen] = ent->base[entlen-1] = 0;\n+\n+\t\t\t/* Detect cases where alternate disappeared */\n+\t\t\tif (stat(ent->base, &st) || !S_ISDIR(st.st_mode)) {\n+\t\t\t\terror(\"object directory %s does not exist; \"\n+\t\t\t\t      \"check .git/objects/info/alternates.\",\n+\t\t\t\t      ent->base);\n+\t\t\t\tgoto bad;\n+\t\t\t}\n+\t\t\tent->base[pfxlen] = '/';\n \n \t\t\t/* Prevent the common mistake of listing the same\n \t\t\t * thing twice, or object directory itself.\n@@ -552,7 +563,9 @@ static void prepare_packed_git_one(char \n \tlen = strlen(path);\n \tdir = opendir(path);\n \tif (!dir) {\n-\t\tfprintf(stderr, \"unable to open object pack directory: %s: %s\\n\", path, strerror(errno));\n+\t\tif (errno != ENOENT)\n+\t\t\terror(\"unable to open object pack directory: %s: %s\\n\",\n+\t\t\t      path, strerror(errno));\n \t\treturn;\n \t}\n \tpath[len++] = '/';\n"},{"id":"16581","messageId":"878xs3rypu.wl%cworth@cworth.org","threadId":"3438","inReplyTo":"7vr75vmcod.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found","fromName":"Carl Worth","fromEmail":"cworth@cworth.org","sentAt":"2006-02-22T19:16:13Z","receivedAt":"2006-02-22T19:16:13Z","isPatch":true,"sender":{"key":"cworth@cworth.org","avatar":"https://gravatar.com/avatar/3746dc28cde609bdbd7f939058356e7e2bbd16d21e32274df0725eb3d998bc5b?d=mp&s=160"},"body":"On Wed, 22 Feb 2006 11:11:14 -0800, Junio C Hamano wrote:\n>\n> Updated patch.  The root cause of the problem you had was that\n> alternates was dangling, so we catch that.\n\nLooks perfect. Thanks for the fix.\n\n-Carl\n"},{"id":"16597","messageId":"20060223003400.GA27800@diku.dk","threadId":"3438","inReplyTo":"7vr75vmcod.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Prevent git-upload-pack segfault if object cannot be found","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2006-02-23T00:34:00Z","receivedAt":"2006-02-23T00:34:00Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Junio C Hamano <junkio@cox.net> wrote Wed, Feb 22, 2006:\n> @@ -552,7 +563,9 @@ static void prepare_packed_git_one(char \n>  \tlen = strlen(path);\n>  \tdir = opendir(path);\n>  \tif (!dir) {\n> -\t\tfprintf(stderr, \"unable to open object pack directory: %s: %s\\n\", path, strerror(errno));\n> +\t\tif (errno != ENOENT)\n> +\t\t\terror(\"unable to open object pack directory: %s: %s\\n\",\n> +\t\t\t      path, strerror(errno));\n\nNo need for the ending \\n.\n\n-- \nJonas Fonseca\n"},{"id":"16600","messageId":"7v64n6luan.fsf_-_@assigned-by-dhcp.cox.net","threadId":"3438","inReplyTo":"20060223003400.GA27800@diku.dk","subject":"[PATCH] Give no terminating LF to error() function.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-23T01:48:16Z","receivedAt":"2006-02-23T01:48:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n  Jonas Fonseca <fonseca@diku.dk> writes:\n\n  > Junio C Hamano <junkio@cox.net> wrote Wed, Feb 22, 2006:\n  >> +\t\t\terror(\"unable to open object pack directory: %s: %s\\n\",\n  >> +\t\t\t      path, strerror(errno));\n  >\n  > No need for the ending \\n.\n\n  True.\n\n commit.c       |    3 ++-\n http-fetch.c   |    8 ++++----\n read-tree.c    |    4 ++--\n receive-pack.c |    4 ++--\n refs.c         |    2 +-\n sha1_file.c    |    7 ++++---\n 6 files changed, 15 insertions(+), 13 deletions(-)\n\n8cd486dcab93856c7eafe33338a3df85b79e4a3e\ndiff --git a/commit.c b/commit.c\nindex c550a00..06d5439 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -212,7 +212,8 @@ int parse_commit_buffer(struct commit *i\n \tif (memcmp(bufptr, \"tree \", 5))\n \t\treturn error(\"bogus commit object %s\", sha1_to_hex(item->object.sha1));\n \tif (get_sha1_hex(bufptr + 5, parent) < 0)\n-\t\treturn error(\"bad tree pointer in commit %s\\n\", sha1_to_hex(item->object.sha1));\n+\t\treturn error(\"bad tree pointer in commit %s\",\n+\t\t\t     sha1_to_hex(item->object.sha1));\n \titem->tree = lookup_tree(parent);\n \tif (item->tree)\n \t\tn_refs++;\ndiff --git a/http-fetch.c b/http-fetch.c\nindex ce3df5f..8fd9de0 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -130,7 +130,7 @@ static void start_object_request(struct \n \n \tif (obj_req->local < 0) {\n \t\tobj_req->state = ABORTED;\n-\t\terror(\"Couldn't create temporary file %s for %s: %s\\n\",\n+\t\terror(\"Couldn't create temporary file %s for %s: %s\",\n \t\t      obj_req->tmpfile, obj_req->filename, strerror(errno));\n \t\treturn;\n \t}\n@@ -830,9 +830,9 @@ static int fetch_object(struct alt_base \n \t\t\t\t    obj_req->errorstr, obj_req->curl_result,\n \t\t\t\t    obj_req->http_code, hex);\n \t} else if (obj_req->zret != Z_STREAM_END) {\n-\t\tret = error(\"File %s (%s) corrupt\\n\", hex, obj_req->url);\n+\t\tret = error(\"File %s (%s) corrupt\", hex, obj_req->url);\n \t} else if (memcmp(obj_req->sha1, obj_req->real_sha1, 20)) {\n-\t\tret = error(\"File %s has bad hash\\n\", hex);\n+\t\tret = error(\"File %s has bad hash\", hex);\n \t} else if (obj_req->rename < 0) {\n \t\tret = error(\"unable to write sha1 filename %s\",\n \t\t\t    obj_req->filename);\n@@ -854,7 +854,7 @@ int fetch(unsigned char *sha1)\n \t\tfetch_alternates(alt->base);\n \t\taltbase = altbase->next;\n \t}\n-\treturn error(\"Unable to find %s under %s\\n\", sha1_to_hex(sha1),\n+\treturn error(\"Unable to find %s under %s\", sha1_to_hex(sha1),\n \t\t     alt->base);\n }\n \ndiff --git a/read-tree.c b/read-tree.c\nindex 52f06e3..8be307e 100644\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -564,7 +564,7 @@ static int twoway_merge(struct cache_ent\n \tstruct cache_entry *oldtree = src[1], *newtree = src[2];\n \n \tif (merge_size != 2)\n-\t\treturn error(\"Cannot do a twoway merge of %d trees\\n\",\n+\t\treturn error(\"Cannot do a twoway merge of %d trees\",\n \t\t\t     merge_size);\n \n \tif (current) {\n@@ -616,7 +616,7 @@ static int oneway_merge(struct cache_ent\n \tstruct cache_entry *a = src[1];\n \n \tif (merge_size != 1)\n-\t\treturn error(\"Cannot do a oneway merge of %d trees\\n\",\n+\t\treturn error(\"Cannot do a oneway merge of %d trees\",\n \t\t\t     merge_size);\n \n \tif (!a)\ndiff --git a/receive-pack.c b/receive-pack.c\nindex eae31e3..2a3db16 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -92,7 +92,7 @@ static int run_update_hook(const char *r\n \tcase -ERR_RUN_COMMAND_WAITPID_WRONG_PID:\n \t\treturn error(\"waitpid is confused\");\n \tcase -ERR_RUN_COMMAND_WAITPID_SIGNAL:\n-\t\treturn error(\"%s died of signal\\n\", update_hook);\n+\t\treturn error(\"%s died of signal\", update_hook);\n \tcase -ERR_RUN_COMMAND_WAITPID_NOEXIT:\n \t\treturn error(\"%s died strangely\", update_hook);\n \tdefault:\n@@ -158,7 +158,7 @@ static int update(struct command *cmd)\n \tif (run_update_hook(name, old_hex, new_hex)) {\n \t\tunlink(lock_name);\n \t\tcmd->error_string = \"hook declined\";\n-\t\treturn error(\"hook declined to update %s\\n\", name);\n+\t\treturn error(\"hook declined to update %s\", name);\n \t}\n \telse if (rename(lock_name, name) < 0) {\n \t\tunlink(lock_name);\ndiff --git a/refs.c b/refs.c\nindex d01fc39..826ae7a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -268,7 +268,7 @@ static int write_ref_file(const char *fi\n \tchar term = '\\n';\n \tif (write(fd, hex, 40) < 40 ||\n \t    write(fd, &term, 1) < 1) {\n-\t\terror(\"Couldn't write %s\\n\", filename);\n+\t\terror(\"Couldn't write %s\", filename);\n \t\tclose(fd);\n \t\treturn -1;\n \t}\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 1fd5b79..a80d849 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -564,7 +564,7 @@ static void prepare_packed_git_one(char \n \tdir = opendir(path);\n \tif (!dir) {\n \t\tif (errno != ENOENT)\n-\t\t\terror(\"unable to open object pack directory: %s: %s\\n\",\n+\t\t\terror(\"unable to open object pack directory: %s: %s\",\n \t\t\t      path, strerror(errno));\n \t\treturn;\n \t}\n@@ -1513,7 +1513,8 @@ int write_sha1_from_fd(const unsigned ch\n \n \tlocal = mkstemp(tmpfile);\n \tif (local < 0)\n-\t\treturn error(\"Couldn't open %s for %s\\n\", tmpfile, sha1_to_hex(sha1));\n+\t\treturn error(\"Couldn't open %s for %s\",\n+\t\t\t     tmpfile, sha1_to_hex(sha1));\n \n \tmemset(&stream, 0, sizeof(stream));\n \n@@ -1561,7 +1562,7 @@ int write_sha1_from_fd(const unsigned ch\n \t}\n \tif (memcmp(sha1, real_sha1, 20)) {\n \t\tunlink(tmpfile);\n-\t\treturn error(\"File %s has bad hash\\n\", sha1_to_hex(sha1));\n+\t\treturn error(\"File %s has bad hash\", sha1_to_hex(sha1));\n \t}\n \n \treturn move_temp_to_file(tmpfile, sha1_file_name(sha1));\n-- \n1.2.3.g5978\n"}]}