Re: [PATCH] real_path: make real_path thread-safe

2016-12-08 Thread Brandon Williams
On 12/08, Johannes Sixt wrote: > Am 08.12.2016 um 08:55 schrieb Torsten Bögershausen: > >Some conversion may be done in mingw.c: > >https://github.com/github/git-msysgit/blob/master/compat/mingw.c > >So what I understand, '/' in Git are already converted into '\' if needed ? > > Only if needed,

Re: [PATCH] real_path: make real_path thread-safe

2016-12-08 Thread Johannes Sixt
Am 08.12.2016 um 08:55 schrieb Torsten Bögershausen: Some conversion may be done in mingw.c: https://github.com/github/git-msysgit/blob/master/compat/mingw.c So what I understand, '/' in Git are already converted into '\' if needed ? Only if needed, and there are not many places where this is

Re: [PATCH] real_path: make real_path thread-safe

2016-12-08 Thread Brandon Williams
On 12/08, Duy Nguyen wrote: > On Tue, Dec 6, 2016 at 3:16 AM, Brandon Williams wrote: > > On 12/05, Stefan Beller wrote: > >> > static const char *real_path_internal(const char *path, int > >> > die_on_error) > >> > { > >> > - static struct strbuf sb = STRBUF_INIT; >

Re: [PATCH] real_path: make real_path thread-safe

2016-12-08 Thread Junio C Hamano
Johannes Sixt writes: > Am 07.12.2016 um 23:29 schrieb Brandon Williams: >> Instead of assuming root is "/" >> I'll need to extract what root is from an absolute path. Aside from >> what root looks like, do most other path constructs behave similarly in >> unix and windows? (like

Re: [PATCH] real_path: make real_path thread-safe

2016-12-08 Thread Johannes Sixt
Am 07.12.2016 um 23:29 schrieb Brandon Williams: Instead of assuming root is "/" I'll need to extract what root is from an absolute path. Aside from what root looks like, do most other path constructs behave similarly in unix and windows? (like ".." and "." as examples) Yes, .. and . work the

Re: [PATCH] real_path: make real_path thread-safe

2016-12-08 Thread Duy Nguyen
On Tue, Dec 6, 2016 at 3:16 AM, Brandon Williams wrote: > On 12/05, Stefan Beller wrote: >> > static const char *real_path_internal(const char *path, int die_on_error) >> > { >> > - static struct strbuf sb = STRBUF_INIT; >> > + static struct strbuf resolved =

Re: [PATCH] real_path: make real_path thread-safe

2016-12-07 Thread Torsten Bögershausen
On Wed, Dec 07, 2016 at 02:13:35PM -0800, Brandon Williams wrote: > On 12/07, Junio C Hamano wrote: > > Torsten Bögershausen writes: > > > > > But in any case it seems that e.g. > > > //SEFVER/SHARE/DIR1/DIR2/.. > > > must be converted into > > > //SEFVER/SHARE/DIR1 > > > > > >

Re: [PATCH] real_path: make real_path thread-safe

2016-12-07 Thread Brandon Williams
On 12/07, Johannes Sixt wrote: > Am 07.12.2016 um 01:10 schrieb Brandon Williams: > >This function should accept both absolute and relative paths, which > >means it should probably accept "C:\My Files". I wasn't thinking about > >windows 100% of the time while writing this so I'm hoping that a

Re: [PATCH] real_path: make real_path thread-safe

2016-12-07 Thread Brandon Williams
On 12/07, Junio C Hamano wrote: > Torsten Bögershausen writes: > > > But in any case it seems that e.g. > > //SEFVER/SHARE/DIR1/DIR2/.. > > must be converted into > > //SEFVER/SHARE/DIR1 > > > > and > > \\SEFVER\SHARE\DIR1\DIR2\.. > > must be converted into > >

Re: [PATCH] real_path: make real_path thread-safe

2016-12-07 Thread Johannes Sixt
Am 07.12.2016 um 01:10 schrieb Brandon Williams: This function should accept both absolute and relative paths, which means it should probably accept "C:\My Files". I wasn't thinking about windows 100% of the time while writing this so I'm hoping that a windows expert will point things like this

Re: [PATCH] real_path: make real_path thread-safe

2016-12-07 Thread Junio C Hamano
Torsten Bögershausen writes: > But in any case it seems that e.g. > //SEFVER/SHARE/DIR1/DIR2/.. > must be converted into > //SEFVER/SHARE/DIR1 > > and > \\SEFVER\SHARE\DIR1\DIR2\.. > must be converted into > \\SEFVER\SHARE\DIR1 Additional questions that may be interesting are:

Re: [PATCH] real_path: make real_path thread-safe

2016-12-07 Thread Torsten Bögershausen
On Wed, Dec 07, 2016 at 01:12:25AM +, Ramsay Jones wrote: > > > On 07/12/16 00:10, Brandon Williams wrote: > > On 12/06, Junio C Hamano wrote: > >> POSIX cares about treating "//" at the very beginning of the path > >> specially. Is that supposed to be handled here, or by a lot higher > >>

Re: [PATCH] real_path: make real_path thread-safe

2016-12-06 Thread Ramsay Jones
On 07/12/16 00:10, Brandon Williams wrote: > On 12/06, Junio C Hamano wrote: >> POSIX cares about treating "//" at the very beginning of the path >> specially. Is that supposed to be handled here, or by a lot higher >> level up in the callchain? > > What exactly does "//" mean in this context?

Re: [PATCH] real_path: make real_path thread-safe

2016-12-06 Thread Brandon Williams
On 12/06, Junio C Hamano wrote: > Brandon Williams writes: > > > +/* removes the last path component from 'path' except if 'path' is root */ > > +static void strip_last_component(struct strbuf *path) > > +{ > > + if (path->len > 1) { > > + char *last_slash =

Re: [PATCH] real_path: make real_path thread-safe

2016-12-06 Thread Junio C Hamano
Brandon Williams writes: > +/* removes the last path component from 'path' except if 'path' is root */ > +static void strip_last_component(struct strbuf *path) > +{ > + if (path->len > 1) { > + char *last_slash = find_last_dir_sep(path->buf); > +

Re: [PATCH] real_path: make real_path thread-safe

2016-12-05 Thread Stefan Beller
On Mon, Dec 5, 2016 at 12:12 PM, Brandon Williams wrote: >> > + if (path->len > 1) { >> > + char *last_slash = find_last_dir_sep(path->buf); >> >> What happens when there is no dir_sep? > > There should always be a dir_sep since that only gets run if the

Re: [PATCH] real_path: make real_path thread-safe

2016-12-05 Thread Brandon Williams
On 12/05, Stefan Beller wrote: > > +/* removes the last path component from 'path' except if 'path' is root */ > > +static void strip_last_component(struct strbuf *path) > > +{ > > + if (path->len > 1) { > > + char *last_slash = find_last_dir_sep(path->buf); > > What happens

Re: [PATCH] real_path: make real_path thread-safe

2016-12-05 Thread Brandon Williams
On 12/05, Stefan Beller wrote: > > static const char *real_path_internal(const char *path, int die_on_error) > > { > > - static struct strbuf sb = STRBUF_INIT; > > + static struct strbuf resolved = STRBUF_INIT; > > Also by having this static here, it is not quite thread safe, yet. >

Re: [PATCH] real_path: make real_path thread-safe

2016-12-05 Thread Stefan Beller
> static const char *real_path_internal(const char *path, int die_on_error) > { > - static struct strbuf sb = STRBUF_INIT; > + static struct strbuf resolved = STRBUF_INIT; Also by having this static here, it is not quite thread safe, yet. By removing the static here we cannot do

Re: [PATCH] real_path: make real_path thread-safe

2016-12-05 Thread Stefan Beller
On Mon, Dec 5, 2016 at 10:58 AM, Brandon Williams wrote: > The current implementation of real_path uses chdir() in order to resolve > symlinks. Unfortunately this isn't thread-safe as chdir() affects a > process as a whole and not just an individual thread. Instead perform >

[PATCH] real_path: make real_path thread-safe

2016-12-05 Thread Brandon Williams
The current implementation of real_path uses chdir() in order to resolve symlinks. Unfortunately this isn't thread-safe as chdir() affects a process as a whole and not just an individual thread. Instead perform the symlink resolution by hand so that the calls to chdir() can be removed, making