symlinks: remove PATH_MAX limitation
PATCH 1/2 symlinks: remove PATH_MAX limitation
'git checkout' fails if a directory is longer than PATH_MAX, because the lstat_cache in symlinks.c checks if the leading directory exists using PATH_MAX-bounded string operations.
Remove the limitation by using strbuf instead.
---
This fixes a bug on Windows with long paths [1].
[1] https://github.com/msysgit/msysgit/issues/227
cache.h | 8 ++++++-- preload-index.c | 4 ++-- symlinks.c | 63 +++++++++++++++++++++++++-------------------------------- 3 files changed, 36 insertions(+), 39 deletions(-)
cache.h +6 -2
@@ -1090,12 +1090,16 @@ struct checkout {extern int checkout_entry (struct cache_entry *ce,const struct checkout *state,char *topath);struct cache_def { -char path[PATH_MAX +1 ]; -int len; +struct strbuf path;int flags;int track_flags;int prefix_len_stat_func; }; +#define CACHE_DEF_INIT { STRBUF_INIT, 0, 0, 0 } +static inline void cache_def_free (struct cache_def *cache) +{ +strbuf_release (&cache->path); +}extern int has_symlink_leading_path (const char *name,int len);extern int threaded_has_symlink_leading_path (struct cache_def *,const char *,int );
preload-index.c +2 -2
@@ -37,9 +37,8 @@ static void *preload_thread(void *_data)struct thread_data *p = _data;struct index_state *index = p->index;struct cache_entry **cep = index->cache + p->offset; -struct cache_def cache; +struct cache_def cache =CACHE_DEF_INIT ; -memset (&cache,0 ,sizeof (cache)); nr = p->nr;if (nr + p->offset > index->cache_nr) nr = index->cache_nr - p->offset;
@@ -64,6 +63,7 @@ static void *preload_thread(void *_data)continue ;ce_mark_uptodate (ce); }while (--nr >0 ); +cache_def_free (&cache);return NULL ; }
symlinks.c +28 -35
141 lines not on this page · raw message
Signature · 21 lines
1.9.4.msysgit.0.5.g1471ac1 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH 2/2 dir: remove PATH_MAX limitation
'git status' segfaults if a directory is longer than PATH_MAX, because processing .gitignore files in prep_exclude() writes past the end of a PATH_MAX-bounded buffer.
Remove the limitation by using strbuf instead.
Note: this fix just 'abuses' strbuf as string allocator, len is always 0. prep_exclude() can probably be simplified using more strbuf APIs.
--- dir.c | 35 +++++++++++++++++++---------------- dir.h | 4 ++-- 2 files changed, 21 insertions(+), 18 deletions(-)
dir.c +19 -16
@@ -798,7 +798,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)* path being checked. */ while ((stk = dir->exclude_stack) !=NULL ) {if (stk->baselen <= baselen && - !strncmp (dir->basebuf, base, stk->baselen)) + !strncmp (dir->base.buf, base, stk->baselen))break ; el = &group->el[dir->exclude_stack->exclude_ix]; dir->exclude_stack = stk->prev;
@@ -833,48 +833,50 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen) stk->baselen = cp - base; stk->exclude_ix = group->nr; el =add_exclude_list (dir,EXC_DIRS ,NULL ); -memcpy (dir->basebuf + current, base + current, +strbuf_grow (&dir->base, stk->baselen); +memcpy (dir->base.buf + current, base + current, stk->baselen - current);/* Abort if the directory is excluded */ if (stk->baselen) {int dt =DT_DIR ; - dir->basebuf[stk->baselen -1 ] =0 ; + dir->base.buf[stk->baselen -1 ] =0 ; dir->exclude =last_exclude_matching_from_lists (dir, - dir->basebuf, stk->baselen -1 , - dir->basebuf + current, &dt); - dir->basebuf[stk->baselen -1 ] ='/' ; + dir->base.buf, stk->baselen -1 , + dir->base.buf + current, &dt); + dir->base.buf[stk->baselen -1 ] ='/' ;if (dir->exclude && dir->exclude->flags &EXC_FLAG_NEGATIVE ) dir->exclude =NULL ;if (dir->exclude) { - dir->basebuf[stk->baselen] =0 ; + dir->base.buf[stk->baselen] =0 ; dir->exclude_stack = stk;return ; } } -/* Try to read per-directory file unless path is too long */ -if (dir->exclude_per_dir && - stk->baselen +strlen (dir->exclude_per_dir) <PATH_MAX ) { -strcpy (dir->basebuf + stk->baselen, +/* Try to read per-directory file */ +if (dir->exclude_per_dir) { +strbuf_grow (&dir->base, stk->baselen + +strlen (dir->exclude_per_dir)); +strcpy (dir->base.buf + stk->baselen, dir->exclude_per_dir);/* -* dir->basebuf gets reused by the traversal, but we +* dir->base gets reused by the traversal, but we * need fname to remain unchanged to ensure the src * member of each struct exclude correctly * back-references its source file. Other invocations * of add_exclude_list provide stable strings, so we * strdup() and free() here in the caller. */ - el->src =strdup (dir->basebuf); -add_excludes_from_file_to_list (dir->basebuf, - dir->basebuf, stk->baselen, el,1 ); + el->src =strdup (dir->base.buf); +add_excludes_from_file_to_list (dir->base.buf, + dir->base.buf, stk->baselen, el,1 ); } dir->exclude_stack = stk; current = stk->baselen; } - dir->basebuf[baselen] ='\0' ; + dir->base.buf[baselen] ='\0' ; }/*
@@ -1671,4 +1673,5 @@ void clear_directory(struct dir_struct *dir)free (stk); stk = prev; } +strbuf_release (&dir->base); }
dir.h +2 -2
@@ -111,13 +111,13 @@ struct dir_struct {* per-directory exclude lists. * * exclude_stack points to the top of the exclude_stack, and -* basebuf contains the full path to the current +* base contains the full path to the current * (sub)directory in the traversal. Exclude points to the * matching exclude struct if the directory is excluded. */ struct exclude_stack *exclude_stack;struct exclude *exclude; -char basebuf[PATH_MAX ]; +struct strbuf base; };/*
Signature · 21 lines
1.9.4.msysgit.0.5.g1471ac1 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
Quoted 8 lines · Karsten Blees
'git status' segfaults if a directory is longer than
PATH_MAX, because processing .gitignore files inprep_exclude()writes past the end of a PATH_MAX-bounded buffer.Remove the limitation by using strbuf instead.
Note: this fix just 'abuses' strbuf as string allocator, len is always 0.
prep_exclude()can probably be simplified using more strbuf APIs.
FYI I had a similar patch [1] that attempted to lazily strbuf_init() instead so that strbuf_ API could be used.
[1] http://article.gmane.org/gmane.comp.version-control.git/248310
Signature · 21 lines
Duy -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
Am 05.07.2014 12:48, schrieb Duy Nguyen:
Quoted 7 lines · Karsten Blees
Quoted 6 lines
'git status' segfaults if a directory is longer than
PATH_MAX, because processing .gitignore files inprep_exclude()writes past the end of a PATH_MAX-bounded buffer.Remove the limitation by using strbuf instead.
Quoted 8 lines
Quoted 2 lines
Note: this fix just 'abuses' strbuf as string allocator, len is always 0.
prep_exclude()can probably be simplified using more strbuf APIs.FYI I had a similar patch [1] that attempted to lazily
strbuf_init()instead so thatstrbuf_API could be used.[1] http://article.gmane.org/gmane.comp.version-control.git/248310
Sorry, I missed that one.
In my version, strbuf_grow() does the lazy initialization (in fact, many strbuf_* functions can handle memset(0) strbufs just fine).
I was simply too lazy to understand (again) how prep_exclude works exactly, and as string calculations like that have the tendency to be just 1 char off, I went for the obviously correct solution (i.e. s/dir->basebuf/dir->base.buf/ plus strbuf_grow() before we write the buffer).
But IMO your version is much cleaner already.
However, api-strbuf.txt says that buf != NULL is invariant after init, and alloc is "somehow private" :-) , so perhaps you should
- if (!dir->basebuf.alloc)
+ if (!dir->basebuf.buf)
strbuf_init(&dir->basebuf, PATH_MAX);
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
In reply to Karsten BleesQuoted 2 lines · Karsten Blees
Note: this fix just 'abuses' strbuf as string allocator, len is always 0.
prep_exclude()can probably be simplified using more strbuf APIs.
Hrm. It looks like you grow it and add some data, but really don't want the length to expand (because the caller depends on it).
In other directory-traversal code we follow a pattern like:
size_t prefix_len = dir->base.len; strbuf_add(&dir->base, cp, stk->baselen - current); /* use full path in dir->base, then "pop" */ strbuf_setlen(&dir->base, stk->baselen);
That makes it a little more obvious that the memcpy matches the strbuf_grow (because it all happens inside strbuf_add).
Is it possible to do something like that here?
-Peff
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
In reply to Karsten BleesQuoted 8 lines · Karsten Blees
'git status' segfaults if a directory is longer than
PATH_MAX, because processing .gitignore files inprep_exclude()writes past the end of a PATH_MAX-bounded buffer.Remove the limitation by using strbuf instead.
Note: this fix just 'abuses' strbuf as string allocator, len is always 0.
prep_exclude()can probably be simplified using more strbuf APIs.
In addition to what Jeff already said, I am afraid that this may make things a lot worse for normal case. By sizing the strbuf to absolute minimum as you dig deeper at each level, wouldn't you copy and reallocate the buffer a lot more, compared to the case where everything fits in the original buffer?
Quoted 115 lines
Signed-off-by Karsten Blees <[email protected]>--- dir.c | 35 +++++++++++++++++++---------------- dir.h | 4 ++-- 2 files changed, 21 insertions(+), 18 deletions(-)dir.c +19
-16@@ -798,7 +798,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)* path being checked. */ while ((stk = dir->exclude_stack) !=NULL ) {if (stk->baselen <= baselen && - !strncmp (dir->basebuf, base, stk->baselen)) + !strncmp (dir->base.buf, base, stk->baselen))break ; el = &group->el[dir->exclude_stack->exclude_ix]; dir->exclude_stack = stk->prev;@@ -833,48 +833,50 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen) stk->baselen = cp - base; stk->exclude_ix = group->nr; el =add_exclude_list (dir,EXC_DIRS ,NULL ); -memcpy (dir->basebuf + current, base + current, +strbuf_grow (&dir->base, stk->baselen); +memcpy (dir->base.buf + current, base + current, stk->baselen - current);/* Abort if the directory is excluded */ if (stk->baselen) {int dt =DT_DIR ; - dir->basebuf[stk->baselen -1 ] =0 ; + dir->base.buf[stk->baselen -1 ] =0 ; dir->exclude =last_exclude_matching_from_lists (dir, - dir->basebuf, stk->baselen -1 , - dir->basebuf + current, &dt); - dir->basebuf[stk->baselen -1 ] ='/' ; + dir->base.buf, stk->baselen -1 , + dir->base.buf + current, &dt); + dir->base.buf[stk->baselen -1 ] ='/' ;if (dir->exclude && dir->exclude->flags &EXC_FLAG_NEGATIVE ) dir->exclude =NULL ;if (dir->exclude) { - dir->basebuf[stk->baselen] =0 ; + dir->base.buf[stk->baselen] =0 ; dir->exclude_stack = stk;return ; } } -/* Try to read per-directory file unless path is too long */ -if (dir->exclude_per_dir && - stk->baselen +strlen (dir->exclude_per_dir) <PATH_MAX ) { -strcpy (dir->basebuf + stk->baselen, +/* Try to read per-directory file */ +if (dir->exclude_per_dir) { +strbuf_grow (&dir->base, stk->baselen + +strlen (dir->exclude_per_dir)); +strcpy (dir->base.buf + stk->baselen, dir->exclude_per_dir);/* -* dir->basebuf gets reused by the traversal, but we +* dir->base gets reused by the traversal, but we * need fname to remain unchanged to ensure the src * member of each struct exclude correctly * back-references its source file. Other invocations * of add_exclude_list provide stable strings, so we * strdup() and free() here in the caller. */ - el->src =strdup (dir->basebuf); -add_excludes_from_file_to_list (dir->basebuf, - dir->basebuf, stk->baselen, el,1 ); + el->src =strdup (dir->base.buf); +add_excludes_from_file_to_list (dir->base.buf, + dir->base.buf, stk->baselen, el,1 ); } dir->exclude_stack = stk; current = stk->baselen; } - dir->basebuf[baselen] ='\0' ; + dir->base.buf[baselen] ='\0' ; }/* @@ -1671,4 +1673,5 @@ void clear_directory(struct dir_struct *dir)free (stk); stk = prev; } +strbuf_release (&dir->base); }dir.h +2
-2@@ -111,13 +111,13 @@ struct dir_struct {* per-directory exclude lists. * * exclude_stack points to the top of the exclude_stack, and -* basebuf contains the full path to the current +* base contains the full path to the current * (sub)directory in the traversal. Exclude points to the * matching exclude struct if the directory is excluded. */ struct exclude_stack *exclude_stack;struct exclude *exclude; -char basebuf[PATH_MAX ]; +struct strbuf base; };/* Signature · 3 lines
1.9.4.msysgit.0.5.g1471ac1 --
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
Am 09.07.2014 18:33, schrieb Junio C Hamano:
Quoted 8 lines
Quoted 6 lines · Karsten Blees
'git status' segfaults if a directory is longer than
PATH_MAX, because processing .gitignore files inprep_exclude()writes past the end of a PATH_MAX-bounded buffer.Remove the limitation by using strbuf instead.
Quoted 9 lines
Quoted 2 lines
Note: this fix just 'abuses' strbuf as string allocator, len is always 0.
prep_exclude()can probably be simplified using more strbuf APIs.In addition to what Jeff already said, I am afraid that this may make things a lot worse for normal case. By sizing the strbuf to absolute minimum as you dig deeper at each level, wouldn't you copy and reallocate the buffer a lot more, compared to the case where everything fits in the original buffer?
strbuf uses ALLOC_GROW, which resizes in (x+16)*1.5 steps (i.e. 24, 60, 114, 195, 316...). Max path len in git is 85, and linux and WebKit are 170ish. I don't think three or four extra reallocs per directory traversal will be noticeable.
Anyways, I'd like to kindly withdraw this patch in favor of Duy's version.
http://article.gmane.org/gmane.comp.version-control.git/248310
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
Quoted 3 lines · Karsten Blees
Anyways, I'd like to kindly withdraw this patch in favor of Duy's version.
http://article.gmane.org/gmane.comp.version-control.git/248310
Thanks; I've already reverted it from 'next'.
Is Duy's patch still viable?
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
Am 12.07.2014 00:29, schrieb Junio C Hamano:
Quoted 10 lines
Quoted 3 lines · Karsten Blees
Anyways, I'd like to kindly withdraw this patch in favor of Duy's version.
http://article.gmane.org/gmane.comp.version-control.git/248310
Thanks; I've already reverted it from 'next'.
Is Duy's patch still viable?
I think so. It fixes the segfault with long paths on Windows as well (Tested-by: <me>), uses strbuf APIs as Peff suggested, and initializes the strbuf with PATH_MAX (i.e. no reallocs in the common case either ;-) ).
AFAICT the first two patches of that series are also completely unrelated to the untracked-cache, so we may want to fast-track these?
[01/20] dir.c: coding style fix
[02/20] dir.h: move struct exclude declaration to top level [03/20] prep_exclude: remove the artificial PATH_MAX limit
...perhaps with s/if (!dir->basebuf.alloc)/if (!dir->basebuf.buf)/
@Duy any reason for not signing off that series?
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
Quoted 19 lines · Karsten Blees
Am 12.07.2014 00:29, schrieb Junio C Hamano:
Quoted 10 lines
Quoted 3 lines · Karsten Blees
Anyways, I'd like to kindly withdraw this patch in favor of Duy's version.
http://article.gmane.org/gmane.comp.version-control.git/248310
Thanks; I've already reverted it from 'next'.
Is Duy's patch still viable?
I think so. It fixes the segfault with long paths on Windows as well (Tested-by: <me>), uses strbuf APIs as Peff suggested, and initializes the strbuf with
PATH_MAX(i.e. no reallocs in the common case either ;-) ).AFAICT the first two patches of that series are also completely unrelated to the untracked-cache, so we may want to fast-track these?
Quoted 7 lines
[01/20]
dir.c: coding style fix
[02/20]dir.h: move struct exclude declaration to top level [03/20]prep_exclude: remove the artificialPATH_MAXlimit...perhaps with s/if (!dir->basebuf.alloc)/if (!dir->basebuf.buf)/
@Duy any reason for not signing off that series?
That series still need a lot more work, but for those first three, if you want to fast track, you have my sign-offs.
Signature · 21 lines
Duy -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
Quoted 10 lines · Duy Nguyen
Quoted 7 lines
[01/20]
dir.c: coding style fix
[02/20]dir.h: move struct exclude declaration to top level [03/20]prep_exclude: remove the artificialPATH_MAXlimit...perhaps with s/if (!dir->basebuf.alloc)/if (!dir->basebuf.buf)/
@Duy any reason for not signing off that series?
That series still need a lot more work, but for those first three, if you want to fast track, you have my sign-offs.
If it is not too much trouble, can you send only the relevant patches (these three) with sign-off again? I'd prefer to give patches a new chance to be eyeballed by people, and they will tend to have a better chance by being a smaller and an isolated topic.
Thanks.
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Hi,
Quoted 4 lines · Karsten Blees
'git checkout' fails if a directory is longer than
PATH_MAX, because thelstat_cacheinsymlinks.cchecks if the leading directory exists using PATH_MAX-bounded string operations.
Quoted 8 lines
Remove the limitation by using strbuf instead.
Signed-off-by Karsten Blees <[email protected]>---
This fixes a bug on Windows with long paths [1].
For context: on Windows, PATH_MAX is ridiculously small. And it really is that small, file functions of the Win32 API cannot handle longer paths.
However, there is a (pretty ugly, if you ask me) workaround: prefixing the paths with "\\?\" and lo and behold, *many* (but not all) Win32 API functions can then handle paths up to something around 32,000 characters!
The problem Karsten addressed here is caused exactly by this oddity: we *can* handle paths much longer than PATH_MAX. Therefore, we need to decouple the path buffers from that hardcoded limitation.
Karsten, you rock.
Ciao,
Dscho
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Quoted 5 lines · Karsten Blees
'git checkout' fails if a directory is longer than
PATH_MAX, because thelstat_cacheinsymlinks.cchecks if the leading directory exists using PATH_MAX-bounded string operations.Remove the limitation by using strbuf instead.
Good.
Quoted 10 lines
cache.h +0
-2@@ -1090,12 +1090,16 @@ struct checkout {extern int checkout_entry (struct cache_entry *ce,const struct checkout *state,char *topath);struct cache_def { -char path[PATH_MAX +1 ]; -int len;
cache.h struct checkout { · line 1102
Quoted 10 lines
cache.h +6
-0+struct strbuf path;int flags;int track_flags;int prefix_len_stat_func; }; +#define CACHE_DEF_INIT { STRBUF_INIT, 0, 0, 0 } +static inline void cache_def_free (struct cache_def *cache) +{ +strbuf_release (&cache->path); +}
The above cache_def_free(cache) does not free the cache itself, but only its associated data, so the name cache_def_free() is somewhat misleading.
It seems that we use the name that consists solely of "something" and "free" if the pointer to "something" itself is also freed. For example "diff_free_filespec_data(struct diff_filespec *)" frees data associated with the filespec but not the filespec itself, which is done with "diff_free_filespec()".
Another name that may not be misleading would be cache_def_clear(); I think I'd prefer it over cache_def_free_data().
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Am 07.07.2014 20:30, schrieb Junio C Hamano:
Quoted 6 lines
Karsten Blees <[email protected]> writes:
The above
cache_def_free(cache)does not free the cache itself, but only its associated data, so the namecache_def_free()is somewhat misleading.
You already merged this to master ("kb/path-max-must-go" lol), should I send a fixup! s/cache_def_free/cache_def_clear/ or is it OK as is?
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Quoted 10 lines · Karsten Blees
Am 07.07.2014 20:30, schrieb Junio C Hamano:
Quoted 6 lines
Karsten Blees <[email protected]> writes:
The above
cache_def_free(cache)does not free the cache itself, but only its associated data, so the namecache_def_free()is somewhat misleading.You already merged this to master ("kb/path-max-must-go" lol), should I send a fixup! s/cache_def_free/cache_def_clear/ or is it OK as is?
I do not think a fix-up would hurt other topics in flight, so please do so.
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH 1/1 fixup! symlinks: remove PATH_MAX limitation
Rename cache_def_free to cache_def_clear as it doesn't free the struct cache_def, but just clears its content.
--- cache.h | 2 +- preload-index.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
cache.h +1 -1
@@ -1096,7 +1096,7 @@ struct cache_def {int prefix_len_stat_func; };#define CACHE_DEF_INIT { STRBUF_INIT, 0, 0, 0 } -static inline void cache_def_free (struct cache_def *cache) +static inline void cache_def_clear (struct cache_def *cache) {strbuf_release (&cache->path); }
preload-index.c +1 -1
@@ -63,7 +63,7 @@ static void *preload_thread(void *_data)continue ;ce_mark_uptodate (ce); }while (--nr >0 ); -cache_def_free (&cache); +cache_def_clear (&cache);return NULL ; }
Signature · 21 lines
2.0.1.563.g66f467c.dirty -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.