{"thread":{"id":"10967","subject":"[PATCH] Fix segmentation fault when user doesn't have access permission to the repository.","startedAt":"2007-11-22T00:59:00Z","lastAt":"2007-12-06T14:09:22Z","messageCount":10,"participants":["André Goddard Rosa","Alex Riesen","Junio C Hamano","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"60586","messageId":"b8bf37780711211659i4d621533o6a3b97349bb75f8c@mail.gmail.com","threadId":"10967","inReplyTo":null,"subject":"[PATCH] Fix segmentation fault when user doesn't have access permission to the repository.","fromName":"André Goddard Rosa","fromEmail":"andre.goddard@gmail.com","sentAt":"2007-11-22T00:59:00Z","receivedAt":"2007-11-22T00:59:00Z","isPatch":true,"sender":{"key":"andre.goddard@gmail.com","avatar":null},"body":"Hi, all!\n\n Please cc: me as I'm not subscribed. I'm sending the patch inline\nonly for review, probably it is mangled.\n Please use the attached patch if you agree with it. Sorry about\nsending it attached.\n\n>From b2af9e783e7d8974b969c01f7a2de07b9cd5cf70 Mon Sep 17 00:00:00 2001\nFrom: Andre Goddard Rosa \nDate: Tue, 27 Nov 2007 10:14:57 -0200\nSubject: [PATCH] Fix segmentation fault when user doesn't have access\npermission to the repository.\n\nWhen trying to \"git-pull\" with my personal user in a tree owned by root,\ngit was crashing with segmentation fault.\n\nSigned-off-by: Andre Goddard Rosa \n---\n builtin-fetch--tool.c | 12 ++++++++++--\n builtin-fetch.c | 14 +++++++++++---\n 2 files changed, 21 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex ed60847..7460ab7 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -511,10 +511,14 @@ int cmd_fetch__tool(int argc, const char **argv,\nconst char *prefix)\n \tif (!strcmp(\"append-fetch-head\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n\n \t\tif (argc != 8)\n \t\t\treturn error(\"append-fetch-head takes 6 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = append_fetch_head(fp, argv[2], argv[3],\n \t\t\t\t\t argv[4], argv[5],\n \t\t\t\t\t argv[6], !!argv[7][0],\n@@ -525,10 +529,14 @@ int cmd_fetch__tool(int argc, const char **argv,\nconst char *prefix)\n \tif (!strcmp(\"native-store\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n\n \t\tif (argc != 5)\n \t\t\treturn error(\"fetch-native-store takes 3 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = fetch_native_store(fp, argv[2], argv[3], argv[4],\n \t\t\t\t\t verbose, force);\n \t\tfclose(fp);\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex be9e3ea..5909d2f 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -263,8 +263,11 @@ static void store_updated_refs(const char *url,\nstruct ref *ref_map)\n \tchar note[1024];\n \tconst char *what, *kind;\n \tstruct ref *rm;\n+\tchar *filename = git_path(\"FETCH_HEAD\");\n\n-\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\tfp = fopen(filename, \"a\");\n+\tif (!fp)\n+\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \tfor (rm = ref_map; rm; rm = rm->next) {\n \t\tstruct ref *ref = NULL;\n\n@@ -487,8 +490,13 @@ static int do_fetch(struct transport *transport,\n \t\tdie(\"Don't know how to fetch from %s\", transport->url);\n\n \t/* if not appending, truncate FETCH_HEAD */\n-\tif (!append)\n-\t\tfclose(fopen(git_path(\"FETCH_HEAD\"), \"w\"));\n+\tif (!append) {\n+\t\tchar *filename = git_path(\"FETCH_HEAD\");\n+\t\tint fd = fopen(filename, \"w\");\n+\t\tif (!fd)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+\t\tfclose(fd);\n+\t}\n\n \tref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);\n\n-- \n1.5.3.6.861.gd794-dirty\n\n\nFrom b2af9e783e7d8974b969c01f7a2de07b9cd5cf70 Mon Sep 17 00:00:00 2001\nFrom: Andre Goddard Rosa \nDate: Tue, 27 Nov 2007 10:14:57 -0200\nSubject: [PATCH] Fix segmentation fault when user doesn't have access permission to the repository.\n\nWhen trying to \"git-pull\" with my personal user in a tree owned by root,\ngit was crashing with segmentation fault.\n\nSigned-off-by: Andre Goddard Rosa \n---\n builtin-fetch--tool.c | 12 ++++++++++--\n builtin-fetch.c | 14 +++++++++++---\n 2 files changed, 21 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex ed60847..7460ab7 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -511,10 +511,14 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(\"append-fetch-head\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n \n \t\tif (argc != 8)\n \t\t\treturn error(\"append-fetch-head takes 6 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = append_fetch_head(fp, argv[2], argv[3],\n \t\t\t\t\t argv[4], argv[5],\n \t\t\t\t\t argv[6], !!argv[7][0],\n@@ -525,10 +529,14 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(\"native-store\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n \n \t\tif (argc != 5)\n \t\t\treturn error(\"fetch-native-store takes 3 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = fetch_native_store(fp, argv[2], argv[3], argv[4],\n \t\t\t\t\t verbose, force);\n \t\tfclose(fp);\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex be9e3ea..5909d2f 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -263,8 +263,11 @@ static void store_updated_refs(const char *url, struct ref *ref_map)\n \tchar note[1024];\n \tconst char *what, *kind;\n \tstruct ref *rm;\n+\tchar *filename = git_path(\"FETCH_HEAD\");\n \n-\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\tfp = fopen(filename, \"a\");\n+\tif (!fp)\n+\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \tfor (rm = ref_map; rm; rm = rm->next) {\n \t\tstruct ref *ref = NULL;\n \n@@ -487,8 +490,13 @@ static int do_fetch(struct transport *transport,\n \t\tdie(\"Don't know how to fetch from %s\", transport->url);\n \n \t/* if not appending, truncate FETCH_HEAD */\n-\tif (!append)\n-\t\tfclose(fopen(git_path(\"FETCH_HEAD\"), \"w\"));\n+\tif (!append) {\n+\t\tchar *filename = git_path(\"FETCH_HEAD\");\n+\t\tint fd = fopen(filename, \"w\");\n+\t\tif (!fd)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+\t\tfclose(fd);\n+\t}\n \n \tref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);\n \n-- \n1.5.3.6.861.gd794-dirty\n\n"},{"id":"60691","messageId":"20071122160959.GA3411@steel.home","threadId":"10967","inReplyTo":"b8bf37780711211659i4d621533o6a3b97349bb75f8c@mail.gmail.com","subject":"Re: [PATCH] Fix segmentation fault when user doesn't have access permission to the repository.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-22T16:09:59Z","receivedAt":"2007-11-22T16:09:59Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"André Goddard Rosa, Thu, Nov 22, 2007 01:59:00 +0100:\n> @@ -487,8 +490,13 @@ static int do_fetch(struct transport *transport,\n> \t\tdie(\"Don't know how to fetch from %s\", transport->url);\n> \n> \t/* if not appending, truncate FETCH_HEAD */\n> -\tif (!append)\n> -\t\tfclose(fopen(git_path(\"FETCH_HEAD\"), \"w\"));\n> +\tif (!append) {\n> +\t\tchar *filename = git_path(\"FETCH_HEAD\");\n> +\t\tint fd = fopen(filename, \"w\");\n\nThis should have been \"FILE *fp\", not \"int fd\".\n\n> +\t\tif (!fd)\n> +\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n> +\t\tfclose(fd);\n> +\t}\n> \n> \tref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);\n> \n> -- \n> 1.5.3.6.861.gd794-dirty\n> \n"},{"id":"60717","messageId":"b8bf37780711221427q5dda709dt38ce1837c0e56c1f@mail.gmail.com","threadId":"10967","inReplyTo":"20071122160959.GA3411@steel.home","subject":"Re: [PATCH] Fix segmentation fault when user doesn't have access permission to the repository.","fromName":"André Goddard Rosa","fromEmail":"andre.goddard@gmail.com","sentAt":"2007-11-22T22:27:59Z","receivedAt":"2007-11-22T22:27:59Z","isPatch":true,"sender":{"key":"andre.goddard@gmail.com","avatar":null},"body":"On Nov 22, 2007 2:09 PM, Alex Riesen wrote:\n> André Goddard Rosa, Thu, Nov 22, 2007 01:59:00 +0100:\n> > @@ -487,8 +490,13 @@ static int do_fetch(struct transport *transport,\n> > die(\"Don't know how to fetch from %s\", transport->url);\n> >\n> > /* if not appending, truncate FETCH_HEAD */\n> > - if (!append)\n> > - fclose(fopen(git_path(\"FETCH_HEAD\"), \"w\"));\n> > + if (!append) {\n> > + char *filename = git_path(\"FETCH_HEAD\");\n> > + int fd = fopen(filename, \"w\");\n>\n> This should have been \"FILE *fp\", not \"int fd\".\n>\n\nHi, Alex!\n\nMany thanks, you're right.\n\nI tested it here before posting but luckly (or not, as I didn't catch\nthis when compiling) it worked,\nas a pointer have the sizeof(int) in my x86 platform. >:|\n\nWould you please comment on the attached patch and see if it's ok?\n\nFrom dbadc5213b9957fb575c6da8528e5dd7a3f1f43e Mon Sep 17 00:00:00 2001\nFrom: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= \nDate: Thu, 22 Nov 2007 20:22:23 -0200\nSubject: [PATCH] Fix segmentation fault when user doesn't have access\n permission to the repository.\nMIME-Version: 1.0\nContent-Type: text/plain; charset=utf-8\nContent-Transfer-Encoding: 8bit\n\nSigned-off-by: André Goddard Rosa \n---\n builtin-fetch--tool.c | 12 ++++++++++--\n builtin-fetch.c | 21 ++++++++++++++++-----\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex ed60847..7460ab7 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -511,10 +511,14 @@ int cmd_fetch__tool(int argc, const char **argv,\nconst char *prefix)\n \tif (!strcmp(\"append-fetch-head\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n\n \t\tif (argc != 8)\n \t\t\treturn error(\"append-fetch-head takes 6 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = append_fetch_head(fp, argv[2], argv[3],\n \t\t\t\t\t argv[4], argv[5],\n \t\t\t\t\t argv[6], !!argv[7][0],\n@@ -525,10 +529,14 @@ int cmd_fetch__tool(int argc, const char **argv,\nconst char *prefix)\n \tif (!strcmp(\"native-store\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n\n \t\tif (argc != 5)\n \t\t\treturn error(\"fetch-native-store takes 3 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = fetch_native_store(fp, argv[2], argv[3], argv[4],\n \t\t\t\t\t verbose, force);\n \t\tfclose(fp);\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex be9e3ea..84c8ed4 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -255,7 +255,7 @@ static int update_local_ref(struct ref *ref,\n \t}\n }\n\n-static void store_updated_refs(const char *url, struct ref *ref_map)\n+static int store_updated_refs(const char *url, struct ref *ref_map)\n {\n \tFILE *fp;\n \tstruct commit *commit;\n@@ -263,8 +263,13 @@ static void store_updated_refs(const char *url,\nstruct ref *ref_map)\n \tchar note[1024];\n \tconst char *what, *kind;\n \tstruct ref *rm;\n+\tchar *filename = git_path(\"FETCH_HEAD\");\n\n-\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\tfp = fopen(filename, \"a\");\n+\tif (!fp) {\n+\t\terror(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+\t\treturn 1;\n+\t}\n \tfor (rm = ref_map; rm; rm = rm->next) {\n \t\tstruct ref *ref = NULL;\n\n@@ -335,6 +340,7 @@ static void store_updated_refs(const char *url,\nstruct ref *ref_map)\n \t\t}\n \t}\n \tfclose(fp);\n+\treturn 0;\n }\n\n /*\n@@ -404,7 +410,7 @@ static int fetch_refs(struct transport *transport,\nstruct ref *ref_map)\n \tif (ret)\n \t\tret = transport_fetch_refs(transport, ref_map);\n \tif (!ret)\n-\t\tstore_updated_refs(transport->url, ref_map);\n+\t\tret |= store_updated_refs(transport->url, ref_map);\n \ttransport_unlock_pack(transport);\n \treturn ret;\n }\n@@ -487,8 +493,13 @@ static int do_fetch(struct transport *transport,\n \t\tdie(\"Don't know how to fetch from %s\", transport->url);\n\n \t/* if not appending, truncate FETCH_HEAD */\n-\tif (!append)\n-\t\tfclose(fopen(git_path(\"FETCH_HEAD\"), \"w\"));\n+\tif (!append) {\n+\t\tchar *filename = git_path(\"FETCH_HEAD\");\n+\t\tFILE *fp = fopen(filename, \"w\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+\t\tfclose(fp);\n+\t}\n\n \tref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);\n\n-- \n1.5.3.6.861.gd794-dirty\n\n\nFrom dbadc5213b9957fb575c6da8528e5dd7a3f1f43e Mon Sep 17 00:00:00 2001\nFrom: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= \nDate: Thu, 22 Nov 2007 20:22:23 -0200\nSubject: [PATCH] Fix segmentation fault when user doesn't have access\n permission to the repository.\nMIME-Version: 1.0\nContent-Type: text/plain; charset=utf-8\nContent-Transfer-Encoding: 8bit\n\nSigned-off-by: André Goddard Rosa \n---\n builtin-fetch--tool.c | 12 ++++++++++--\n builtin-fetch.c | 21 ++++++++++++++++-----\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex ed60847..7460ab7 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -511,10 +511,14 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(\"append-fetch-head\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n \n \t\tif (argc != 8)\n \t\t\treturn error(\"append-fetch-head takes 6 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = append_fetch_head(fp, argv[2], argv[3],\n \t\t\t\t\t argv[4], argv[5],\n \t\t\t\t\t argv[6], !!argv[7][0],\n@@ -525,10 +529,14 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(\"native-store\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n \n \t\tif (argc != 5)\n \t\t\treturn error(\"fetch-native-store takes 3 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = fetch_native_store(fp, argv[2], argv[3], argv[4],\n \t\t\t\t\t verbose, force);\n \t\tfclose(fp);\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex be9e3ea..84c8ed4 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -255,7 +255,7 @@ static int update_local_ref(struct ref *ref,\n \t}\n }\n \n-static void store_updated_refs(const char *url, struct ref *ref_map)\n+static int store_updated_refs(const char *url, struct ref *ref_map)\n {\n \tFILE *fp;\n \tstruct commit *commit;\n@@ -263,8 +263,13 @@ static void store_updated_refs(const char *url, struct ref *ref_map)\n \tchar note[1024];\n \tconst char *what, *kind;\n \tstruct ref *rm;\n+\tchar *filename = git_path(\"FETCH_HEAD\");\n \n-\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\tfp = fopen(filename, \"a\");\n+\tif (!fp) {\n+\t\terror(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+\t\treturn 1;\n+\t}\n \tfor (rm = ref_map; rm; rm = rm->next) {\n \t\tstruct ref *ref = NULL;\n \n@@ -335,6 +340,7 @@ static void store_updated_refs(const char *url, struct ref *ref_map)\n \t\t}\n \t}\n \tfclose(fp);\n+\treturn 0;\n }\n \n /*\n@@ -404,7 +410,7 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n \tif (ret)\n \t\tret = transport_fetch_refs(transport, ref_map);\n \tif (!ret)\n-\t\tstore_updated_refs(transport->url, ref_map);\n+\t\tret |= store_updated_refs(transport->url, ref_map);\n \ttransport_unlock_pack(transport);\n \treturn ret;\n }\n@@ -487,8 +493,13 @@ static int do_fetch(struct transport *transport,\n \t\tdie(\"Don't know how to fetch from %s\", transport->url);\n \n \t/* if not appending, truncate FETCH_HEAD */\n-\tif (!append)\n-\t\tfclose(fopen(git_path(\"FETCH_HEAD\"), \"w\"));\n+\tif (!append) {\n+\t\tchar *filename = git_path(\"FETCH_HEAD\");\n+\t\tFILE *fp = fopen(filename, \"w\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+\t\tfclose(fp);\n+\t}\n \n \tref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);\n \n-- \n1.5.3.6.861.gd794-dirty\n\n"},{"id":"60872","messageId":"b8bf37780711251339y796286fbj2cd8d9225008e13@mail.gmail.com","threadId":"10967","inReplyTo":"b8bf37780711221427q5dda709dt38ce1837c0e56c1f@mail.gmail.com","subject":"[Resend PATCH] Fix segmentation fault when user doesn't have access permission to the repository.","fromName":"André Goddard Rosa","fromEmail":"andre.goddard@gmail.com","sentAt":"2007-11-25T21:39:10Z","receivedAt":"2007-11-25T21:39:10Z","isPatch":true,"sender":{"key":"andre.goddard@gmail.com","avatar":null},"body":"On Nov 22, 2007 2:09 PM, Alex Riesen wrote:\n> André Goddard Rosa, Thu, Nov 22, 2007 01:59:00 +0100:\n> > @@ -487,8 +490,13 @@ static int do_fetch(struct transport *transport,\n> > die(\"Don't know how to fetch from %s\", transport->url);\n> >\n> > /* if not appending, truncate FETCH_HEAD */\n> > - if (!append)\n> > - fclose(fopen(git_path(\"FETCH_HEAD\"), \"w\"));\n> > + if (!append) {\n> > + char *filename = git_path(\"FETCH_HEAD\");\n> > + int fd = fopen(filename, \"w\");\n>\n> This should have been \"FILE *fp\", not \"int fd\".\n>\n\nHi, Alex!\n\nMany thanks, you're right.\n\nI tested it here before posting but luckly (or not, as I didn't catch\nthis when compiling) it worked,\nas a pointer have the sizeof(int) in my x86 platform. >:|\n\nWould you please comment on the attached patch and see if it's ok?\n\nFrom dbadc5213b9957fb575c6da8528e5dd7a3f1f43e Mon Sep 17 00:00:00 2001\nFrom: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= \nDate: Thu, 22 Nov 2007 20:22:23 -0200\nSubject: [PATCH] Fix segmentation fault when user doesn't have access\n permission to the repository.\nMIME-Version: 1.0\nContent-Type: text/plain; charset=utf-8\nContent-Transfer-Encoding: 8bit\n\nSigned-off-by: André Goddard Rosa \n---\n builtin-fetch--tool.c | 12 ++++++++++--\n builtin-fetch.c | 21 ++++++++++++++++-----\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex ed60847..7460ab7 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -511,10 +511,14 @@ int cmd_fetch__tool(int argc, const char **argv,\nconst char *prefix)\n if (!strcmp(\"append-fetch-head\", argv[1])) {\n int result;\n FILE *fp;\n+ char *filename;\n\n if (argc != 8)\n return error(\"append-fetch-head takes 6 args\");\n- fp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+ filename = git_path(\"FETCH_HEAD\");\n+ fp = fopen(filename, \"a\");\n+ if (!fp)\n+ return error(\"cannot open %s: %s\\n\", filename,\nstrerror(errno));\n result = append_fetch_head(fp, argv[2], argv[3],\n argv[4], argv[5],\n argv[6], !!argv[7][0],\n@@ -525,10 +529,14 @@ int cmd_fetch__tool(int argc, const char **argv,\nconst char *prefix)\n if (!strcmp(\"native-store\", argv[1])) {\n int result;\n FILE *fp;\n+ char *filename;\n\n if (argc != 5)\n return error(\"fetch-native-store takes 3 args\");\n- fp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+ filename = git_path(\"FETCH_HEAD\");\n+ fp = fopen(filename, \"a\");\n+ if (!fp)\n+ return error(\"cannot open %s: %s\\n\", filename,\nstrerror(errno));\n result = fetch_native_store(fp, argv[2], argv[3], argv[4],\n verbose, force);\n fclose(fp);\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex be9e3ea..84c8ed4 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -255,7 +255,7 @@ static int update_local_ref(struct ref *ref,\n }\n }\n\n-static void store_updated_refs(const char *url, struct ref *ref_map)\n+static int store_updated_refs(const char *url, struct ref *ref_map)\n {\n FILE *fp;\n struct commit *commit;\n@@ -263,8 +263,13 @@ static void store_updated_refs(const char *url,\nstruct ref *ref_map)\n char note[1024];\n const char *what, *kind;\n struct ref *rm;\n+ char *filename = git_path(\"FETCH_HEAD\");\n\n- fp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+ fp = fopen(filename, \"a\");\n+ if (!fp) {\n+ error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+ return 1;\n+ }\n for (rm = ref_map; rm; rm = rm->next) {\n struct ref *ref = NULL;\n\n@@ -335,6 +340,7 @@ static void store_updated_refs(const char *url,\nstruct ref *ref_map)\n }\n }\n fclose(fp);\n+ return 0;\n }\n\n /*\n@@ -404,7 +410,7 @@ static int fetch_refs(struct transport *transport,\nstruct ref *ref_map)\n if (ret)\n ret = transport_fetch_refs(transport, ref_map);\n if (!ret)\n- store_updated_refs(transport->url, ref_map);\n+ ret |= store_updated_refs(transport->url, ref_map);\n transport_unlock_pack(transport);\n return ret;\n }\n@@ -487,8 +493,13 @@ static int do_fetch(struct transport *transport,\n die(\"Don't know how to fetch from %s\", transport->url);\n\n /* if not appending, truncate FETCH_HEAD */\n- if (!append)\n- fclose(fopen(git_path(\"FETCH_HEAD\"), \"w\"));\n+ if (!append) {\n+ char *filename = git_path(\"FETCH_HEAD\");\n+ FILE *fp = fopen(filename, \"w\");\n+ if (!fp)\n+ return error(\"cannot open %s: %s\\n\", filename,\nstrerror(errno));\n+ fclose(fp);\n\n+ }\n\n ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);\n\n--\n1.5.3.6.861.gd794-dirty\n\n\n\n-- \n[]s,\nAndré Goddard\n\n\nFrom dbadc5213b9957fb575c6da8528e5dd7a3f1f43e Mon Sep 17 00:00:00 2001\nFrom: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= \nDate: Thu, 22 Nov 2007 20:22:23 -0200\nSubject: [PATCH] Fix segmentation fault when user doesn't have access\n permission to the repository.\nMIME-Version: 1.0\nContent-Type: text/plain; charset=utf-8\nContent-Transfer-Encoding: 8bit\n\nSigned-off-by: André Goddard Rosa \n---\n builtin-fetch--tool.c | 12 ++++++++++--\n builtin-fetch.c | 21 ++++++++++++++++-----\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex ed60847..7460ab7 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -511,10 +511,14 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(\"append-fetch-head\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n \n \t\tif (argc != 8)\n \t\t\treturn error(\"append-fetch-head takes 6 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = append_fetch_head(fp, argv[2], argv[3],\n \t\t\t\t\t argv[4], argv[5],\n \t\t\t\t\t argv[6], !!argv[7][0],\n@@ -525,10 +529,14 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(\"native-store\", argv[1])) {\n \t\tint result;\n \t\tFILE *fp;\n+\t\tchar *filename;\n \n \t\tif (argc != 5)\n \t\t\treturn error(\"fetch-native-store takes 3 args\");\n-\t\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\t\tfilename = git_path(\"FETCH_HEAD\");\n+\t\tfp = fopen(filename, \"a\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n \t\tresult = fetch_native_store(fp, argv[2], argv[3], argv[4],\n \t\t\t\t\t verbose, force);\n \t\tfclose(fp);\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex be9e3ea..84c8ed4 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -255,7 +255,7 @@ static int update_local_ref(struct ref *ref,\n \t}\n }\n \n-static void store_updated_refs(const char *url, struct ref *ref_map)\n+static int store_updated_refs(const char *url, struct ref *ref_map)\n {\n \tFILE *fp;\n \tstruct commit *commit;\n@@ -263,8 +263,13 @@ static void store_updated_refs(const char *url, struct ref *ref_map)\n \tchar note[1024];\n \tconst char *what, *kind;\n \tstruct ref *rm;\n+\tchar *filename = git_path(\"FETCH_HEAD\");\n \n-\tfp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n+\tfp = fopen(filename, \"a\");\n+\tif (!fp) {\n+\t\terror(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+\t\treturn 1;\n+\t}\n \tfor (rm = ref_map; rm; rm = rm->next) {\n \t\tstruct ref *ref = NULL;\n \n@@ -335,6 +340,7 @@ static void store_updated_refs(const char *url, struct ref *ref_map)\n \t\t}\n \t}\n \tfclose(fp);\n+\treturn 0;\n }\n \n /*\n@@ -404,7 +410,7 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n \tif (ret)\n \t\tret = transport_fetch_refs(transport, ref_map);\n \tif (!ret)\n-\t\tstore_updated_refs(transport->url, ref_map);\n+\t\tret |= store_updated_refs(transport->url, ref_map);\n \ttransport_unlock_pack(transport);\n \treturn ret;\n }\n@@ -487,8 +493,13 @@ static int do_fetch(struct transport *transport,\n \t\tdie(\"Don't know how to fetch from %s\", transport->url);\n \n \t/* if not appending, truncate FETCH_HEAD */\n-\tif (!append)\n-\t\tfclose(fopen(git_path(\"FETCH_HEAD\"), \"w\"));\n+\tif (!append) {\n+\t\tchar *filename = git_path(\"FETCH_HEAD\");\n+\t\tFILE *fp = fopen(filename, \"w\");\n+\t\tif (!fp)\n+\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+\t\tfclose(fp);\n+\t}\n \n \tref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);\n \n-- \n1.5.3.6.861.gd794-dirty\n\n"},{"id":"61524","messageId":"7v3aunqvha.fsf_-_@gitster.siamese.dyndns.org","threadId":"10967","inReplyTo":"b8bf37780711251339y796286fbj2cd8d9225008e13@mail.gmail.com","subject":"Re:* [Resend PATCH] Fix segmentation fault when user doesn't have access permission to the repository.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-30T21:22:41Z","receivedAt":"2007-11-30T21:22:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"André Goddard Rosa\" writes:\n\n> On Nov 22, 2007 2:09 PM, Alex Riesen wrote:\n> ...\n> I tested it here before posting but luckly (or not, as I didn't catch\n> this when compiling) it worked,\n> as a pointer have the sizeof(int) in my x86 platform. >:|\n\nMost of the changes look trivially correct. Thanks.\n\n> diff --git a/builtin-fetch.c b/builtin-fetch.c\n> index be9e3ea..84c8ed4 100644\n> --- a/builtin-fetch.c\n> +++ b/builtin-fetch.c\n> @@ -263,8 +263,13 @@ static void store_updated_refs(const char *url,\n> struct ref *ref_map)\n> char note[1024];\n> const char *what, *kind;\n> struct ref *rm;\n> + char *filename = git_path(\"FETCH_HEAD\");\n>\n> - fp = fopen(git_path(\"FETCH_HEAD\"), \"a\");\n> + fp = fopen(filename, \"a\");\n> + if (!fp) {\n> + error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n> + return 1;\n> + }\n> for (rm = ref_map; rm; rm = rm->next) {\n> struct ref *ref = NULL;\n\nI started to wonder if there was a particular reason you chose to return\n1, not -1 (or just say 'return error(\"cannot open...\", ...)')?\n\n> @@ -404,7 +410,7 @@ static int fetch_refs(struct transport *transport,\n> struct ref *ref_map)\n> if (ret)\n> ret = transport_fetch_refs(transport, ref_map);\n> if (!ret)\n> - store_updated_refs(transport->url, ref_map);\n> + ret |= store_updated_refs(transport->url, ref_map);\n> transport_unlock_pack(transport);\n> return ret;\n> }\n\nI think the callers of fetch_refs() are interested in seeing only 0 or\nnon-zero, so it seems more consistent to signal error with negative\nreturn. I modified the hunk that starts at line 263 to return the\nreturn value from error() and applied.\n\nThat made me follow the transport code, fetch_refs_via_pack(). This is\nnot your code, so Shawn and Daniel are CC'ed.\n\nThe code calls fetch_pack() to get the list of refs it fetched, and\ndiscards refs and always returns 0 to signal success.\n\nBut builtin-fetch-pack.c::fetch_pack() has error cases. The function\nreturns NULL if error is detected (shallow-support side seems to choose\nto die but I suspect that is easily fixable to error out as well).\n\nShouldn't fetch_refs_via_pack() propagate that error to the caller?\n\n---\n transport.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 50db980..048df1f 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -655,7 +655,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \tfree(heads);\n \tfree_refs(refs);\n \tfree(dest);\n-\treturn 0;\n+\treturn (refs ? 0 : -1);\n }\n \n static int git_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)\n"},{"id":"61973","messageId":"7vk5nt1v7k.fsf@gitster.siamese.dyndns.org","threadId":"10967","inReplyTo":"7v3aunqvha.fsf_-_@gitster.siamese.dyndns.org","subject":"fetch_refs_via_pack() discards status?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-05T07:01:19Z","receivedAt":"2007-12-05T07:01:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The code calls fetch_pack() to get the list of refs it fetched, and\ndiscards refs and always returns 0 to signal success.\n\nBut builtin-fetch-pack.c::fetch_pack() has error cases. The function\nreturns NULL if error is detected (shallow-support side seems to choose\nto die but I suspect that is easily fixable to error out as well).\n\nShouldn't fetch_refs_via_pack() propagate that error to the caller?\n\n---\n transport.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 50db980..048df1f 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -655,7 +655,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \tfree(heads);\n \tfree_refs(refs);\n \tfree(dest);\n-\treturn 0;\n+\treturn (refs ? 0 : -1);\n }\n \n static int git_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)\n"},{"id":"62043","messageId":"Pine.LNX.4.64.0712051356040.5349@iabervon.org","threadId":"10967","inReplyTo":"7vk5nt1v7k.fsf@gitster.siamese.dyndns.org","subject":"Re: fetch_refs_via_pack() discards status?","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-12-05T19:16:04Z","receivedAt":"2007-12-05T19:16:04Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 4 Dec 2007, Junio C Hamano wrote:\n\n> The code calls fetch_pack() to get the list of refs it fetched, and\n> discards refs and always returns 0 to signal success.\n> \n> But builtin-fetch-pack.c::fetch_pack() has error cases. The function\n> returns NULL if error is detected (shallow-support side seems to choose\n> to die but I suspect that is easily fixable to error out as well).\n> \n> Shouldn't fetch_refs_via_pack() propagate that error to the caller?\n\nI think that's right. I think I got as far as having the error status from \nfetch_pack() actually returned correctly, and then failed to look at it. \nI'd personally avoid testing a pointer to freed memory, but that's \nobviously not actually wrong.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"62054","messageId":"7vwsrsonqm.fsf@gitster.siamese.dyndns.org","threadId":"10967","inReplyTo":"Pine.LNX.4.64.0712051356040.5349@iabervon.org","subject":"Re: fetch_refs_via_pack() discards status?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-05T21:06:25Z","receivedAt":"2007-12-05T21:06:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> On Tue, 4 Dec 2007, Junio C Hamano wrote:\n>\n>> The code calls fetch_pack() to get the list of refs it fetched, and\n>> discards refs and always returns 0 to signal success.\n>> \n>> But builtin-fetch-pack.c::fetch_pack() has error cases. The function\n>> returns NULL if error is detected (shallow-support side seems to choose\n>> to die but I suspect that is easily fixable to error out as well).\n>> \n>> Shouldn't fetch_refs_via_pack() propagate that error to the caller?\n>\n> I think that's right. I think I got as far as having the error status from \n> fetch_pack() actually returned correctly, and then failed to look at it. \n> I'd personally avoid testing a pointer to freed memory, but that's \n> obviously not actually wrong.\n>\n> \t-Daniel\n\nHmph, is that an Ack that the patchlet is actually a bugfix?\n"},{"id":"62076","messageId":"b8bf37780712051638x5a2ea5d4i61631f31af65cb54@mail.gmail.com","threadId":"10967","inReplyTo":"7vwsrsonqm.fsf@gitster.siamese.dyndns.org","subject":"Re: fetch_refs_via_pack() discards status?","fromName":"André Goddard Rosa","fromEmail":"andre.goddard@gmail.com","sentAt":"2007-12-06T00:38:35Z","receivedAt":"2007-12-06T00:38:35Z","isPatch":false,"sender":{"key":"andre.goddard@gmail.com","avatar":null},"body":"On Dec 5, 2007 7:06 PM, Junio C Hamano wrote:\n> Daniel Barkalow writes:\n>\n> > On Tue, 4 Dec 2007, Junio C Hamano wrote:\n> >\n> >> The code calls fetch_pack() to get the list of refs it fetched, and\n> >> discards refs and always returns 0 to signal success.\n> >>\n> >> But builtin-fetch-pack.c::fetch_pack() has error cases. The function\n> >> returns NULL if error is detected (shallow-support side seems to choose\n> >> to die but I suspect that is easily fixable to error out as well).\n> >>\n> >> Shouldn't fetch_refs_via_pack() propagate that error to the caller?\n> >\n> > I think that's right. I think I got as far as having the error status from\n> > fetch_pack() actually returned correctly, and then failed to look at it.\n> > I'd personally avoid testing a pointer to freed memory, but that's\n> > obviously not actually wrong.\n> >\n> > -Daniel\n>\n> Hmph, is that an Ack that the patchlet is actually a bugfix?\n>\n\nHi, Mr. Junio!\n\n My 2 cents: I think he means that we should not test a freed pointer:\n\n free_refs(refs);\n free(dest); <===\n- return 0;\n+ return (refs ? 0 : -1); <===\n\nBest regards,\n-- \n[]s,\nAndré Goddard\n"},{"id":"62148","messageId":"Pine.LNX.4.64.0712060907420.5349@iabervon.org","threadId":"10967","inReplyTo":"7vwsrsonqm.fsf@gitster.siamese.dyndns.org","subject":"Re: fetch_refs_via_pack() discards status?","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-12-06T14:09:22Z","receivedAt":"2007-12-06T14:09:22Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 5 Dec 2007, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> > On Tue, 4 Dec 2007, Junio C Hamano wrote:\n> >\n> >> The code calls fetch_pack() to get the list of refs it fetched, and\n> >> discards refs and always returns 0 to signal success.\n> >> \n> >> But builtin-fetch-pack.c::fetch_pack() has error cases. The function\n> >> returns NULL if error is detected (shallow-support side seems to choose\n> >> to die but I suspect that is easily fixable to error out as well).\n> >> \n> >> Shouldn't fetch_refs_via_pack() propagate that error to the caller?\n> >\n> > I think that's right. I think I got as far as having the error status from \n> > fetch_pack() actually returned correctly, and then failed to look at it. \n> > I'd personally avoid testing a pointer to freed memory, but that's \n> > obviously not actually wrong.\n> >\n> > \t-Daniel\n> \n> Hmph, is that an Ack that the patchlet is actually a bugfix?\n\nYes.\n\nAcked-By: Daniel Barkalow \n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}