{"thread":{"id":"28432","subject":"[PATCH] abspath: increase array size of cwd variable to PATH_MAX","startedAt":"2011-09-19T09:51:12Z","lastAt":"2011-09-22T08:54:16Z","messageCount":6,"participants":["Wang Hui","Junio C Hamano","Ramsay Jones","wanghui"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"175781","messageId":"1316425872-30457-1-git-send-email-Hui.Wang@windriver.com","threadId":"28432","inReplyTo":null,"subject":"[PATCH] abspath: increase array size of cwd variable to PATH_MAX","fromName":"Wang Hui","fromEmail":"hui.wang@windriver.com","sentAt":"2011-09-19T09:51:12Z","receivedAt":"2011-09-19T09:51:12Z","isPatch":true,"sender":{"key":"hui.wang@windriver.com","avatar":null},"body":"From: Hui Wang \n\nIf the name length of working dir exceeds 1024 characters, neither git\nclone nor git init can succeed under the working dir.\nE.G. %>for ((i=1;i<300;i++));do mkdir 1234567890;cd 1234567890;done\n %>git clone ~/git\n fatal: Could not get current working directory: Numerical result\n out of range\n\nThis is because both git clone and git init will call\nabspath.c:real_path(), in the real_path(), it will call getcwd()\nto get and save current working dir, here we passed a 1024 char size\narray to the parameter, if the name length of current working dir\nexceeds 1024, this function will fail.\n\nThe purpose of calling getcwd() is to save current working dir, then\nbefore the real_path() return, restore to the saved dir. We should use\nPATH_MAX instead of 1024 for the array size.\n\nSigned-off-by: Hui Wang \n---\n abspath.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex f04ac18..2ce1db9 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -24,7 +24,7 @@ int is_directory(const char *path)\n const char *real_path(const char *path)\n {\n \tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n-\tchar cwd[1024] = \"\";\n+\tchar cwd[PATH_MAX] = \"\";\n \tint buf_index = 1;\n \n \tint depth = MAXDEPTH;\n-- \n1.6.3.1\n"},{"id":"175787","messageId":"7v8vpkbhyv.fsf@alter.siamese.dyndns.org","threadId":"28432","inReplyTo":"1316425872-30457-1-git-send-email-Hui.Wang@windriver.com","subject":"Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-19T16:43:52Z","receivedAt":"2011-09-19T16:43:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wang Hui writes:\n\n> diff --git a/abspath.c b/abspath.c\n> index f04ac18..2ce1db9 100644\n> --- a/abspath.c\n> +++ b/abspath.c\n> @@ -24,7 +24,7 @@ int is_directory(const char *path)\n> const char *real_path(const char *path)\n> {\n> \tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n> -\tchar cwd[1024] = \"\";\n> +\tchar cwd[PATH_MAX] = \"\";\n\nThanks.\n\nThis does not make things worse but in the longer term we should move away\nfrom using PATH_MAX in general.\n"},{"id":"175953","messageId":"4E791A40.6040102@ramsay1.demon.co.uk","threadId":"28432","inReplyTo":"7v8vpkbhyv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-09-20T22:57:04Z","receivedAt":"2011-09-20T22:57:04Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Wang Hui writes:\n> \n>> diff --git a/abspath.c b/abspath.c\n>> index f04ac18..2ce1db9 100644\n>> --- a/abspath.c\n>> +++ b/abspath.c\n>> @@ -24,7 +24,7 @@ int is_directory(const char *path)\n>> const char *real_path(const char *path)\n>> {\n>> \tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n>> -\tchar cwd[1024] = \"\";\n>> +\tchar cwd[PATH_MAX] = \"\";\n> \n> Thanks.\n> \n> This does not make things worse but in the longer term we should move away\n> from using PATH_MAX in general.\n\nHmm, the subject line says \"... increase array size ...\", but that is not\nnecessarily what this patch is doing! :-D\n\nYes, on some platforms PATH_MAX will be larger than 1024 (e.g. 4096 on Linux),\nbut that is not even true of all POSIX systems. POSIX defines the *minimum*\nvalue of PATH_MAX that systems must support (as #define _POSIX_PATH_MAX) of 255.\n[it also requires that POSIX conforming applications must not *require* a value\nlarger than 255].\n\nHowever, we don't have to look too far to find systems with much smaller values.\nOn Cygwin, for example:\n\n $ cat -n junk.c\n 1 #include \n 2 #include \n 3\n 4 int main(int argc, char *argv[])\n 5 {\n 6 printf(\"PATH_MAX is %d\\n\", PATH_MAX);\n 7 return 0;\n 8 }\n $ gcc -o junk junk.c\n $ ./junk\n $ PATH_MAX is 260\n $ \n\nOn MinGW the answer is 259.\n\nSo, I certainly agree that moving away from PATH_MAX is a good idea, but I'm\nnot sure I agree that this patch \"does not make things worse\" ... (I haven't\ngiven it *any* thought!).\n\n[Also, note commits f66cf96, fd55a19, 620e2bb, etc...]\n\nATB,\nRamsay Jones\n"},{"id":"175962","messageId":"7vty851wg9.fsf@alter.siamese.dyndns.org","threadId":"28432","inReplyTo":"4E791A40.6040102@ramsay1.demon.co.uk","subject":"Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-21T20:17:58Z","receivedAt":"2011-09-21T20:17:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> Hmm, the subject line says \"... increase array size ...\", but that is not\n> necessarily what this patch is doing! :-D\n\nTrue; will revert it out of 'next'.\n\nThanks for noticing.\n"},{"id":"175976","messageId":"4E7A98EC.6040007@windriver.com","threadId":"28432","inReplyTo":"4E791A40.6040102@ramsay1.demon.co.uk","subject":"Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX","fromName":"wanghui","fromEmail":"hui.wang@windriver.com","sentAt":"2011-09-22T02:09:48Z","receivedAt":"2011-09-22T02:09:48Z","isPatch":true,"sender":{"key":"hui.wang@windriver.com","avatar":null},"body":"Ramsay Jones wrote:\n> Junio C Hamano wrote:\n> \n>> Wang Hui writes:\n>>\n>> \n>>> diff --git a/abspath.c b/abspath.c\n>>> index f04ac18..2ce1db9 100644\n>>> --- a/abspath.c\n>>> +++ b/abspath.c\n>>> @@ -24,7 +24,7 @@ int is_directory(const char *path)\n>>> const char *real_path(const char *path)\n>>> {\n>>> \tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n>>> -\tchar cwd[1024] = \"\";\n>>> +\tchar cwd[PATH_MAX] = \"\";\n>>> \n>> Thanks.\n>>\n>> This does not make things worse but in the longer term we should move away\n>> from using PATH_MAX in general.\n>> \n>\n> Hmm, the subject line says \"... increase array size ...\", but that is not\n> necessarily what this patch is doing! :-D\n>\n> Yes, on some platforms PATH_MAX will be larger than 1024 (e.g. 4096 on Linux),\n> but that is not even true of all POSIX systems. POSIX defines the *minimum*\n> value of PATH_MAX that systems must support (as #define _POSIX_PATH_MAX) of 255.\n> [it also requires that POSIX conforming applications must not *require* a value\n> larger than 255].\n>\n> However, we don't have to look too far to find systems with much smaller values.\n> On Cygwin, for example:\n>\n> $ cat -n junk.c\n> 1 #include \n> 2 #include \n> 3\n> 4 int main(int argc, char *argv[])\n> 5 {\n> 6 printf(\"PATH_MAX is %d\\n\", PATH_MAX);\n> 7 return 0;\n> 8 }\n> $ gcc -o junk junk.c\n> $ ./junk\n> $ PATH_MAX is 260\n> $ \n>\n> On MinGW the answer is 259.\n>\n> So, I certainly agree that moving away from PATH_MAX is a good idea, but I'm\n> not sure I agree that this patch \"does not make things worse\" ... (I haven't\n> given it *any* thought!).\n> \nHi Ramsay,\n\nDo you mean the PATH_MAX of a system should not be the limitation for \nthe git. That is to say, the git can handle the path which has name \nlonger than PATH_MAX? If it is, my patch is not needed here. :-)\n> [Also, note commits f66cf96, fd55a19, 620e2bb, etc...]\n>\n> ATB,\n> Ramsay Jones\n>\n>\n> \n"},{"id":"175990","messageId":"4E7AF7B8.2080208@windriver.com","threadId":"28432","inReplyTo":"7vty851wg9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX","fromName":"wanghui","fromEmail":"hui.wang@windriver.com","sentAt":"2011-09-22T08:54:16Z","receivedAt":"2011-09-22T08:54:16Z","isPatch":true,"sender":{"key":"hui.wang@windriver.com","avatar":null},"body":"Junio C Hamano wrote:\n> Ramsay Jones writes:\n>\n> \n>> Hmm, the subject line says \"... increase array size ...\", but that is not\n>> necessarily what this patch is doing! :-D\n>> \n>\n> True; will revert it out of 'next'.\n>\n> Thanks for noticing.\n>\n> \nHi Junio and Ramsay,\n\nIn fact, i found lots of similar usage in the git source code. E.G.\n\nin the dir.c,\nint is_inside_dir(const char *dir)\n{\nchar cwd[PATH_MAX];\nif (!dir)\nreturn 0;\nif (!getcwd(cwd, sizeof(cwd)))\ndie_errno(\"can't find the current directory\");\nreturn dir_inside_of(cwd, dir) >= 0;\n}\n\nSince this implementation is not cross-OS safe, Could we use below solution?\n\nstep1, add a new function xgetcwd() to wrapper getcwd() like this:\nchar *xgetcwd(void)\n{\nsize_t size = 100;\n\nwhile (1) {\nchar *buffer = (char *) xmalloc (size);\nif (getcwd (buffer, size) == buffer)\nreturn buffer;\nfree (buffer);\nif (errno != ERANGE)\nreturn 0;\nsize *= 2;\n}\n}\n\nstep2, use xgetcwd to replace all getcwd occurrence in the git source code.\n\nThis will add a little bit overhead to the git, but it is cross-OS safe.\n"}]}