* [PATCH] abspath: increase array size of cwd variable to PATH_MAX
@ 2011-09-19 9:51 Wang Hui
2011-09-19 16:43 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Wang Hui @ 2011-09-19 9:51 UTC (permalink / raw)
To: gitster, git
From: Hui Wang <Hui.Wang@windriver.com>
If the name length of working dir exceeds 1024 characters, neither git
clone nor git init can succeed under the working dir.
E.G. %>for ((i=1;i<300;i++));do mkdir 1234567890;cd 1234567890;done
%>git clone ~/git
fatal: Could not get current working directory: Numerical result
out of range
This is because both git clone and git init will call
abspath.c:real_path(), in the real_path(), it will call getcwd()
to get and save current working dir, here we passed a 1024 char size
array to the parameter, if the name length of current working dir
exceeds 1024, this function will fail.
The purpose of calling getcwd() is to save current working dir, then
before the real_path() return, restore to the saved dir. We should use
PATH_MAX instead of 1024 for the array size.
Signed-off-by: Hui Wang <Hui.Wang@windriver.com>
---
abspath.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/abspath.c b/abspath.c
index f04ac18..2ce1db9 100644
--- a/abspath.c
+++ b/abspath.c
@@ -24,7 +24,7 @@ int is_directory(const char *path)
const char *real_path(const char *path)
{
static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
- char cwd[1024] = "";
+ char cwd[PATH_MAX] = "";
int buf_index = 1;
int depth = MAXDEPTH;
--
1.6.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX
2011-09-19 9:51 [PATCH] abspath: increase array size of cwd variable to PATH_MAX Wang Hui
@ 2011-09-19 16:43 ` Junio C Hamano
2011-09-20 22:57 ` Ramsay Jones
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2011-09-19 16:43 UTC (permalink / raw)
To: Wang Hui; +Cc: git
Wang Hui <Hui.Wang@windriver.com> writes:
> diff --git a/abspath.c b/abspath.c
> index f04ac18..2ce1db9 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -24,7 +24,7 @@ int is_directory(const char *path)
> const char *real_path(const char *path)
> {
> static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
> - char cwd[1024] = "";
> + char cwd[PATH_MAX] = "";
Thanks.
This does not make things worse but in the longer term we should move away
from using PATH_MAX in general.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX
2011-09-19 16:43 ` Junio C Hamano
@ 2011-09-20 22:57 ` Ramsay Jones
2011-09-21 20:17 ` Junio C Hamano
2011-09-22 2:09 ` wanghui
0 siblings, 2 replies; 6+ messages in thread
From: Ramsay Jones @ 2011-09-20 22:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Wang Hui, git
Junio C Hamano wrote:
> Wang Hui <Hui.Wang@windriver.com> writes:
>
>> diff --git a/abspath.c b/abspath.c
>> index f04ac18..2ce1db9 100644
>> --- a/abspath.c
>> +++ b/abspath.c
>> @@ -24,7 +24,7 @@ int is_directory(const char *path)
>> const char *real_path(const char *path)
>> {
>> static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
>> - char cwd[1024] = "";
>> + char cwd[PATH_MAX] = "";
>
> Thanks.
>
> This does not make things worse but in the longer term we should move away
> from using PATH_MAX in general.
Hmm, the subject line says "... increase array size ...", but that is not
necessarily what this patch is doing! :-D
Yes, on some platforms PATH_MAX will be larger than 1024 (e.g. 4096 on Linux),
but that is not even true of all POSIX systems. POSIX defines the *minimum*
value of PATH_MAX that systems must support (as #define _POSIX_PATH_MAX) of 255.
[it also requires that POSIX conforming applications must not *require* a value
larger than 255].
However, we don't have to look too far to find systems with much smaller values.
On Cygwin, for example:
$ cat -n junk.c
1 #include <stdio.h>
2 #include <limits.h>
3
4 int main(int argc, char *argv[])
5 {
6 printf("PATH_MAX is %d\n", PATH_MAX);
7 return 0;
8 }
$ gcc -o junk junk.c
$ ./junk
$ PATH_MAX is 260
$
On MinGW the answer is 259.
So, I certainly agree that moving away from PATH_MAX is a good idea, but I'm
not sure I agree that this patch "does not make things worse" ... (I haven't
given it *any* thought!).
[Also, note commits f66cf96, fd55a19, 620e2bb, etc...]
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX
2011-09-20 22:57 ` Ramsay Jones
@ 2011-09-21 20:17 ` Junio C Hamano
2011-09-22 8:54 ` wanghui
2011-09-22 2:09 ` wanghui
1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2011-09-21 20:17 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Wang Hui, git
Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> Hmm, the subject line says "... increase array size ...", but that is not
> necessarily what this patch is doing! :-D
True; will revert it out of 'next'.
Thanks for noticing.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX
2011-09-21 20:17 ` Junio C Hamano
@ 2011-09-22 8:54 ` wanghui
0 siblings, 0 replies; 6+ messages in thread
From: wanghui @ 2011-09-22 8:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ramsay Jones, git
Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>
>
>> Hmm, the subject line says "... increase array size ...", but that is not
>> necessarily what this patch is doing! :-D
>>
>
> True; will revert it out of 'next'.
>
> Thanks for noticing.
>
>
Hi Junio and Ramsay,
In fact, i found lots of similar usage in the git source code. E.G.
in the dir.c,
int is_inside_dir(const char *dir)
{
char cwd[PATH_MAX];
if (!dir)
return 0;
if (!getcwd(cwd, sizeof(cwd)))
die_errno("can't find the current directory");
return dir_inside_of(cwd, dir) >= 0;
}
Since this implementation is not cross-OS safe, Could we use below solution?
step1, add a new function xgetcwd() to wrapper getcwd() like this:
char *xgetcwd(void)
{
size_t size = 100;
while (1) {
char *buffer = (char *) xmalloc (size);
if (getcwd (buffer, size) == buffer)
return buffer;
free (buffer);
if (errno != ERANGE)
return 0;
size *= 2;
}
}
step2, use xgetcwd to replace all getcwd occurrence in the git source code.
This will add a little bit overhead to the git, but it is cross-OS safe.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX
2011-09-20 22:57 ` Ramsay Jones
2011-09-21 20:17 ` Junio C Hamano
@ 2011-09-22 2:09 ` wanghui
1 sibling, 0 replies; 6+ messages in thread
From: wanghui @ 2011-09-22 2:09 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, git
Ramsay Jones wrote:
> Junio C Hamano wrote:
>
>> Wang Hui <Hui.Wang@windriver.com> writes:
>>
>>
>>> diff --git a/abspath.c b/abspath.c
>>> index f04ac18..2ce1db9 100644
>>> --- a/abspath.c
>>> +++ b/abspath.c
>>> @@ -24,7 +24,7 @@ int is_directory(const char *path)
>>> const char *real_path(const char *path)
>>> {
>>> static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
>>> - char cwd[1024] = "";
>>> + char cwd[PATH_MAX] = "";
>>>
>> Thanks.
>>
>> This does not make things worse but in the longer term we should move away
>> from using PATH_MAX in general.
>>
>
> Hmm, the subject line says "... increase array size ...", but that is not
> necessarily what this patch is doing! :-D
>
> Yes, on some platforms PATH_MAX will be larger than 1024 (e.g. 4096 on Linux),
> but that is not even true of all POSIX systems. POSIX defines the *minimum*
> value of PATH_MAX that systems must support (as #define _POSIX_PATH_MAX) of 255.
> [it also requires that POSIX conforming applications must not *require* a value
> larger than 255].
>
> However, we don't have to look too far to find systems with much smaller values.
> On Cygwin, for example:
>
> $ cat -n junk.c
> 1 #include <stdio.h>
> 2 #include <limits.h>
> 3
> 4 int main(int argc, char *argv[])
> 5 {
> 6 printf("PATH_MAX is %d\n", PATH_MAX);
> 7 return 0;
> 8 }
> $ gcc -o junk junk.c
> $ ./junk
> $ PATH_MAX is 260
> $
>
> On MinGW the answer is 259.
>
> So, I certainly agree that moving away from PATH_MAX is a good idea, but I'm
> not sure I agree that this patch "does not make things worse" ... (I haven't
> given it *any* thought!).
>
Hi Ramsay,
Do you mean the PATH_MAX of a system should not be the limitation for
the git. That is to say, the git can handle the path which has name
longer than PATH_MAX? If it is, my patch is not needed here. :-)
> [Also, note commits f66cf96, fd55a19, 620e2bb, etc...]
>
> ATB,
> Ramsay Jones
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-22 8:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-19 9:51 [PATCH] abspath: increase array size of cwd variable to PATH_MAX Wang Hui
2011-09-19 16:43 ` Junio C Hamano
2011-09-20 22:57 ` Ramsay Jones
2011-09-21 20:17 ` Junio C Hamano
2011-09-22 8:54 ` wanghui
2011-09-22 2:09 ` wanghui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).