public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
* bad bash tab completion
@ 2012-08-10 14:25 Shaddy Baddah
  2012-08-10 16:54 ` Eric Blake
  2013-01-14  5:21 ` stat() and tilde prefix (was bad bash tab completion) Shaddy Baddah
  0 siblings, 2 replies; 23+ messages in thread
From: Shaddy Baddah @ 2012-08-10 14:25 UTC (permalink / raw)
  To: cygwin

Hi,

I've been having this problem with bash for about half a year now. I
can't remember the specific upgrade that caused it, but that's around
the time frame.

So the issue is with tab completion, and looks something like this:

snapshot 1:

$ cd ~/../

snapshot 2 (after tab-tab):

$ cd \~/../

snapshot 3 (after further tab-tab):

$ cd \~/../
.ssh/     tmp/      workarea/

My main problem is with snapshot 2. By my understanding, the path that
bash has modified to is no longer a valid path. snapshot 3, and further
actions from there back it up, because bash is no longer looking in the
right place.

I have tried this with bash-completions disabled, and the same thing
happens.

I do have a non-conventional setup of my home directory:

$ echo ~
/cygdrive/c/Users/sbaddah/cygwin-home

A hunch of mine is that the contents of ~/../, aka
/cygdrive/c/Users/sbaddah/, are involved. As recent Windows editions
seem to include a lot of "virtual" type folders/links that I've never
quite got my head around/


In lieu of providing a cygcheck.out, which I will do if it is absolutely
necessary. here are the details of my cygwin install:

$ uname -a
CYGWIN_NT-6.1-WOW64 AUD-R87FXYH 1.7.16(0.262/5/3) 2012-07-20 22:55 i686 
Cygwin

$ cygcheck -cd bash
Cygwin Package Information
Package              Version
bash                 4.1.10-4

Any ideas?

-- 
Regards,
Shaddy


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: bad bash tab completion
  2012-08-10 14:25 bad bash tab completion Shaddy Baddah
@ 2012-08-10 16:54 ` Eric Blake
  2013-01-14  5:21 ` stat() and tilde prefix (was bad bash tab completion) Shaddy Baddah
  1 sibling, 0 replies; 23+ messages in thread
From: Eric Blake @ 2012-08-10 16:54 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

On 08/10/2012 01:14 AM, Shaddy Baddah wrote:
> Hi,
> 
> I've been having this problem with bash for about half a year now. I
> can't remember the specific upgrade that caused it, but that's around
> the time frame.
> 
> So the issue is with tab completion, and looks something like this:
> 
> snapshot 1:
> 
> $ cd ~/../
> 
> snapshot 2 (after tab-tab):
> 
> $ cd \~/../

Most likely a bug in upstream bash and/or upstream bash-completion.
Both of these packages are a bit long in the tooth in cygwin; I really
need to find time to upgrade the distro to the latest versions of these
packages, which may fix the issue right there.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* stat() and tilde prefix (was bad bash tab completion)
  2012-08-10 14:25 bad bash tab completion Shaddy Baddah
  2012-08-10 16:54 ` Eric Blake
@ 2013-01-14  5:21 ` Shaddy Baddah
  2013-01-14  6:17   ` Christopher Faylor
  1 sibling, 1 reply; 23+ messages in thread
From: Shaddy Baddah @ 2013-01-14  5:21 UTC (permalink / raw)
  To: cygwin

Hi,

On 10 Aug 2012 17:14, Shaddy Baddah wrote:
> I've been having this problem with bash for about half a year now. I
> can't remember the specific upgrade that caused it, but that's around
> the time frame.
>
> So the issue is with tab completion, and looks something like this:
>
> snapshot 1:
>
> $ cd ~/../
>
> snapshot 2 (after tab-tab):
>
> $ cd \~/../
>
> snapshot 3 (after further tab-tab):
>
> $ cd \~/../
> .ssh/ tmp/ workarea/
>
> My main problem is with snapshot 2. By my understanding, the path that
> bash has modified to is no longer a valid path. snapshot 3, and further
> actions from there back it up, because bash is no longer looking in the
> right place.
>
> I have tried this with bash-completions disabled, and the same thing
> happens.
</snip>
(see http://cygwin.com/ml/cygwin/2012-08/msg00226.html)

Sorry to drag this old email up, but it gives context to what I am about
to write.

In investigating this, I believe the issue I am having is due to how
stat() handles tilde prefixed paths. On linux we see:

linux$ $ python -c 'import os; print os.stat("~/..")'
Traceback (most recent call last):
   File "<string>", line 1, in <module>
OSError: [Errno 2] No such file or directory: '~/..'

and on cygwin we see:

cygwin$ python -c 'import os; print os.stat("~/..")'
posix.stat_result(st_mode=16832, st_ino=562949953496729L, 
st_dev=4174909669L, st_nlink=1, st_uid=42037, st_gid=10513, st_size=0L, 
st_atime=1357616166, st_mtime=1357616166, st_ctime=1357616166)

This make sense as the reason in the context of the bash code which
handles the tab-completion. It is escaping the path as it thinks it
references an actual valid posix path (according to my understanding
that the tilde has no special meaning to the posix api):

http://git.savannah.gnu.org/cgit/bash.git/tree/bashline.c?id=509a4430ae72aec10896713435e84f5b27675763#n3480

       /* XXX -- check for standalone tildes here and backslash-quote 
them */
       if (s == text && *s == '~' && file_exists (text))
         *r++ = '\\';

where file_exists() just wraps stat():

http://git.savannah.gnu.org/cgit/bash.git/tree/general.c?id=509a4430ae72aec10896713435e84f5b27675763#n537

int
file_exists (fn)
      char *fn;
{
   struct stat sb;

   return (stat (fn, &sb) == 0);
}

Going back through various Cygwin dll versions, I see that stat() has
behaved in this way since as early as 1.7.6 if not earlier. I am unsure
if previous versions of bash had special handling for this stat()
behaviour, so it may be that bash has behaved this way for quite a while
longer that I had detected.

Can this be fixed? It seems like a change that may be tricky for other
applications?

-- 
Regards,
Shaddy

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14  5:21 ` stat() and tilde prefix (was bad bash tab completion) Shaddy Baddah
@ 2013-01-14  6:17   ` Christopher Faylor
  2013-01-14 12:23     ` Corinna Vinschen
  0 siblings, 1 reply; 23+ messages in thread
From: Christopher Faylor @ 2013-01-14  6:17 UTC (permalink / raw)
  To: cygwin

On Mon, Jan 14, 2013 at 04:21:25PM +1100, Shaddy Baddah wrote:
>In investigating this, I believe the issue I am having is due to how
>stat() handles tilde prefixed paths. On linux we see:
>
>linux$ $ python -c 'import os; print os.stat("~/..")'
>Traceback (most recent call last):
>   File "<string>", line 1, in <module>
>OSError: [Errno 2] No such file or directory: '~/..'
>
>and on cygwin we see:
>
>cygwin$ python -c 'import os; print os.stat("~/..")'
>posix.stat_result(st_mode=16832, st_ino=562949953496729L, 
>st_dev=4174909669L, st_nlink=1, st_uid=42037, st_gid=10513, st_size=0L, 
>st_atime=1357616166, st_mtime=1357616166, st_ctime=1357616166)

It is a bug.  It's not just "~".  Any nonexistent directory will
work, like "foo/..".

cgf

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14  6:17   ` Christopher Faylor
@ 2013-01-14 12:23     ` Corinna Vinschen
  2013-01-14 14:37       ` Shaddy Baddah
                         ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Corinna Vinschen @ 2013-01-14 12:23 UTC (permalink / raw)
  To: cygwin

On Jan 14 01:17, Christopher Faylor wrote:
> On Mon, Jan 14, 2013 at 04:21:25PM +1100, Shaddy Baddah wrote:
> >In investigating this, I believe the issue I am having is due to how
> >stat() handles tilde prefixed paths. On linux we see:
> >
> >linux$ $ python -c 'import os; print os.stat("~/..")'
> >Traceback (most recent call last):
> >   File "<string>", line 1, in <module>
> >OSError: [Errno 2] No such file or directory: '~/..'
> >
> >and on cygwin we see:
> >
> >cygwin$ python -c 'import os; print os.stat("~/..")'
> >posix.stat_result(st_mode=16832, st_ino=562949953496729L, 
> >st_dev=4174909669L, st_nlink=1, st_uid=42037, st_gid=10513, st_size=0L, 
> >st_atime=1357616166, st_mtime=1357616166, st_ctime=1357616166)
> 
> It is a bug.  It's not just "~".  Any nonexistent directory will
> work, like "foo/..".

And it's a bug which isn't easily fixed.  Since about the dawn of time,
Cygwin's core path handling evaluates the path in a non-POSIX manner,
mainly for performance reasons.

POSIX demands to evaluate a path from left to right (thus tripping over
the non-existant "~" or "foo" directory).  Windows OTOH skips testing
all parent directories of a path, and while this can be changed(*), it's
the default setting since the earliest Windows NT versions.

So, since Cygwin can't rely on the OS to this job when it has to convert
a POSIX path to a Windows path internally, Cygwin would have to check
the existence of every single path component from left to right to
emulate the POSIX requirements.  But that would be a big performance
hit, so Cygwin's path handling code tries to be clever to avoid having
to call too many OS functions:

The first step of converting a POSIX path to a Windows path is to
normalize the path.  "." and ".." components are simply dropped:

  "a/b/./c"  -> "a\b\c"
  "a/b/../c" -> "a\c"

Then the path prefix is replaced by the matching mount point.

Eventually it evaluates the path from right to left.  Consider a valid,
normalized path with 10 components.  Under POSIX rules this requires 10
checks for existence.  No problem for the Linux kernel since it has
everything under control anyway and the test is blazingly fast.

But Cygwin is not the OS so it has to call the necessary OS functions 10
times.  By checking from right to left, Cygwin has to call the OS
functions only once, if the file exists, two times if the file does not
exist, but its parent dir exists, and so on.  On top of that, the entire
chore has to restart when tripping over a symbolic link.

Since the predominant number of file operations are performed on
existing paths, or at least paths for which the parent dir exists,
Cygwin reduces the number of OS operations to convert a POSIX to a
Windows path.  The price we're paying is this very deviation from the
POSIX standard.


Corinna

(*) User right "Bypass Traverse Checking", by default enabled for
    everyone.

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 12:23     ` Corinna Vinschen
@ 2013-01-14 14:37       ` Shaddy Baddah
  2013-01-14 16:13         ` Corinna Vinschen
  2013-01-14 15:27       ` Christopher Faylor
  2013-01-14 22:15       ` Thomas Wolff
  2 siblings, 1 reply; 23+ messages in thread
From: Shaddy Baddah @ 2013-01-14 14:37 UTC (permalink / raw)
  To: cygwin

Hi,

On 14/01/13 21:00, Corinna Vinschen wrote:
> On Jan 14 01:17, Christopher Faylor wrote:
>> On Mon, Jan 14, 2013 at 04:21:25PM +1100, Shaddy Baddah wrote:
>>> In investigating this, I believe the issue I am having is due to how
>>> stat() handles tilde prefixed paths. On linux we see:
>>>
>>> linux$ $ python -c 'import os; print os.stat("~/..")'
>>> Traceback (most recent call last):
>>>    File "<string>", line 1, in<module>
>>> OSError: [Errno 2] No such file or directory: '~/..'
>>>
>>> and on cygwin we see:
>>>
>>> cygwin$ python -c 'import os; print os.stat("~/..")'
>>> posix.stat_result(st_mode=16832, st_ino=562949953496729L,
>>> st_dev=4174909669L, st_nlink=1, st_uid=42037, st_gid=10513, st_size=0L,
>>> st_atime=1357616166, st_mtime=1357616166, st_ctime=1357616166)
>>
>> It is a bug.  It's not just "~".  Any nonexistent directory will
>> work, like "foo/..".
>
> And it's a bug which isn't easily fixed.  Since about the dawn of time,
> Cygwin's core path handling evaluates the path in a non-POSIX manner,
> mainly for performance reasons.

Thank you both for your explanations. If my understanding is correct,
stat() will be returning for the current working directory.

It seems to me then that a patch to bash may be in order? I can see how
the bash check is the right thing to do. It doesn't want the special
tilde expansion to mask and disallow referencing of real tilde prefixed
paths. So the stat() check is the quick win to determine that.

 From what I make of it, there needs to be a patch that, although can
work generically, adds checks only required for Cygwin. And therefore
is specific to the Cygwin package.

The check would be an extension of the file_exists() function, perhaps
called tilde_file_exists(), which determines if the tilde prefix forms
a directory component of the path (strchr('/')?). If it does not, the
file_exists() check is sufficient. If it does, then the check of if
that directory exists is logically and'ed to the result of
file_exists().

Does that sound about right?

-- 
Regards,
Shaddy

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 12:23     ` Corinna Vinschen
  2013-01-14 14:37       ` Shaddy Baddah
@ 2013-01-14 15:27       ` Christopher Faylor
  2013-01-14 16:05         ` Corinna Vinschen
  2013-01-14 22:15       ` Thomas Wolff
  2 siblings, 1 reply; 23+ messages in thread
From: Christopher Faylor @ 2013-01-14 15:27 UTC (permalink / raw)
  To: cygwin

On Mon, Jan 14, 2013 at 11:00:02AM +0100, Corinna Vinschen wrote:
>On Jan 14 01:17, Christopher Faylor wrote:
>> On Mon, Jan 14, 2013 at 04:21:25PM +1100, Shaddy Baddah wrote:
>> >In investigating this, I believe the issue I am having is due to how
>> >stat() handles tilde prefixed paths. On linux we see:
>> >
>> >linux$ $ python -c 'import os; print os.stat("~/..")'
>> >Traceback (most recent call last):
>> >   File "<string>", line 1, in <module>
>> >OSError: [Errno 2] No such file or directory: '~/..'
>> >
>> >and on cygwin we see:
>> >
>> >cygwin$ python -c 'import os; print os.stat("~/..")'
>> >posix.stat_result(st_mode=16832, st_ino=562949953496729L, 
>> >st_dev=4174909669L, st_nlink=1, st_uid=42037, st_gid=10513, st_size=0L, 
>> >st_atime=1357616166, st_mtime=1357616166, st_ctime=1357616166)
>> 
>> It is a bug.  It's not just "~".  Any nonexistent directory will
>> work, like "foo/..".
>
>And it's a bug which isn't easily fixed.  Since about the dawn of time,
>Cygwin's core path handling evaluates the path in a non-POSIX manner,
>mainly for performance reasons.
>
>POSIX demands to evaluate a path from left to right (thus tripping over
>the non-existant "~" or "foo" directory).  Windows OTOH skips testing
>all parent directories of a path, and while this can be changed(*), it's
>the default setting since the earliest Windows NT versions.
>
>So, since Cygwin can't rely on the OS to this job when it has to convert
>a POSIX path to a Windows path internally, Cygwin would have to check
>the existence of every single path component from left to right to
>emulate the POSIX requirements.  But that would be a big performance
>hit, so Cygwin's path handling code tries to be clever to avoid having
>to call too many OS functions:
>
>The first step of converting a POSIX path to a Windows path is to
>normalize the path.  "." and ".." components are simply dropped:
>
>  "a/b/./c"  -> "a\b\c"
>  "a/b/../c" -> "a\c"
>
>Then the path prefix is replaced by the matching mount point.
>
>Eventually it evaluates the path from right to left.  Consider a valid,
>normalized path with 10 components.  Under POSIX rules this requires 10
>checks for existence.  No problem for the Linux kernel since it has
>everything under control anyway and the test is blazingly fast.
>
>But Cygwin is not the OS so it has to call the necessary OS functions 10
>times.  By checking from right to left, Cygwin has to call the OS
>functions only once, if the file exists, two times if the file does not
>exist, but its parent dir exists, and so on.  On top of that, the entire
>chore has to restart when tripping over a symbolic link.
>
>Since the predominant number of file operations are performed on
>existing paths, or at least paths for which the parent dir exists,
>Cygwin reduces the number of OS operations to convert a POSIX to a
>Windows path.  The price we're paying is this very deviation from the
>POSIX standard.

Also:

  c:\>dir foo\bar\..\..

   Volume in drive S is share          Serial number is e620:3c3d
   Directory of  S:\*

   1/11/2013   9:58         <DIR>    .
  12/26/2012  21:34         <DIR>    ..
   1/12/2013  16:27         <DIR>    bin
   1/14/2013  10:20         <DIR>    cgf
   ...

I don't have a foo directory but cmd was happy to just ignore that
fact and show my the root directory.  This is YA place where Windows
and Linux differ drastically.

cgf

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 15:27       ` Christopher Faylor
@ 2013-01-14 16:05         ` Corinna Vinschen
  2013-01-14 17:00           ` Christopher Faylor
  0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2013-01-14 16:05 UTC (permalink / raw)
  To: cygwin

On Jan 14 10:27, Christopher Faylor wrote:
> On Mon, Jan 14, 2013 at 11:00:02AM +0100, Corinna Vinschen wrote:
> >The first step of converting a POSIX path to a Windows path is to
> >normalize the path.  "." and ".." components are simply dropped:
> >
> >  "a/b/./c"  -> "a\b\c"
> >  "a/b/../c" -> "a\c"
> >[...]
> Also:
> 
>   c:\>dir foo\bar\..\..
> 
>    Volume in drive S is share          Serial number is e620:3c3d
>    Directory of  S:\*
> 
>    1/11/2013   9:58         <DIR>    .
>   12/26/2012  21:34         <DIR>    ..
>    1/12/2013  16:27         <DIR>    bin
>    1/14/2013  10:20         <DIR>    cgf
>    ...
> 
> I don't have a foo directory but cmd was happy to just ignore that
> fact and show my the root directory.  This is YA place where Windows
> and Linux differ drastically.

Indeed.  Before writing my mail I tested the "GetFullPathName" function,
and I was not exactly surprised to find that it behaves as you describe
for CMD.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 14:37       ` Shaddy Baddah
@ 2013-01-14 16:13         ` Corinna Vinschen
  2013-01-14 20:29           ` Stephan Mueller
  2013-01-15 12:33           ` Shaddy Baddah
  0 siblings, 2 replies; 23+ messages in thread
From: Corinna Vinschen @ 2013-01-14 16:13 UTC (permalink / raw)
  To: cygwin

On Jan 15 01:36, Shaddy Baddah wrote:
> Hi,
> 
> On 14/01/13 21:00, Corinna Vinschen wrote:
> >On Jan 14 01:17, Christopher Faylor wrote:
> >>On Mon, Jan 14, 2013 at 04:21:25PM +1100, Shaddy Baddah wrote:
> >>>In investigating this, I believe the issue I am having is due to how
> >>>stat() handles tilde prefixed paths. On linux we see:
> >>>
> >>>linux$ $ python -c 'import os; print os.stat("~/..")'
> >>>Traceback (most recent call last):
> >>>   File "<string>", line 1, in<module>
> >>>OSError: [Errno 2] No such file or directory: '~/..'
> >>>
> >>>and on cygwin we see:
> >>>
> >>>cygwin$ python -c 'import os; print os.stat("~/..")'
> >>>posix.stat_result(st_mode=16832, st_ino=562949953496729L,
> >>>st_dev=4174909669L, st_nlink=1, st_uid=42037, st_gid=10513, st_size=0L,
> >>>st_atime=1357616166, st_mtime=1357616166, st_ctime=1357616166)
> >>
> >>It is a bug.  It's not just "~".  Any nonexistent directory will
> >>work, like "foo/..".
> >
> >And it's a bug which isn't easily fixed.  Since about the dawn of time,
> >Cygwin's core path handling evaluates the path in a non-POSIX manner,
> >mainly for performance reasons.
> 
> Thank you both for your explanations. If my understanding is correct,
> stat() will be returning for the current working directory.

In the above case, yes.

> It seems to me then that a patch to bash may be in order? I can see how
> the bash check is the right thing to do. It doesn't want the special
> tilde expansion to mask and disallow referencing of real tilde prefixed
> paths. So the stat() check is the quick win to determine that.
> 
> From what I make of it, there needs to be a patch that, although can
> work generically, adds checks only required for Cygwin. And therefore
> is specific to the Cygwin package.
> 
> The check would be an extension of the file_exists() function, perhaps
> called tilde_file_exists(), which determines if the tilde prefix forms
> a directory component of the path (strchr('/')?). If it does not, the
> file_exists() check is sufficient. If it does, then the check of if
> that directory exists is logically and'ed to the result of
> file_exists().
> 
> Does that sound about right?

A check like this might be a good idea.  Ultimately I would be glad to
be able to come up with more correct code in Cygwin while not getting
slower, of course.  But that's wishful thinking for now.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 16:05         ` Corinna Vinschen
@ 2013-01-14 17:00           ` Christopher Faylor
  0 siblings, 0 replies; 23+ messages in thread
From: Christopher Faylor @ 2013-01-14 17:00 UTC (permalink / raw)
  To: cygwin

On Mon, Jan 14, 2013 at 05:04:17PM +0100, Corinna Vinschen wrote:
>On Jan 14 10:27, Christopher Faylor wrote:
>> On Mon, Jan 14, 2013 at 11:00:02AM +0100, Corinna Vinschen wrote:
>> >The first step of converting a POSIX path to a Windows path is to
>> >normalize the path.  "." and ".." components are simply dropped:
>> >
>> >  "a/b/./c"  -> "a\b\c"
>> >  "a/b/../c" -> "a\c"
>> >[...]
>> Also:
>> 
>>   c:\>dir foo\bar\..\..
>> 
>>    Volume in drive S is share          Serial number is e620:3c3d
>>    Directory of  S:\*
>> 
>>    1/11/2013   9:58         <DIR>    .
>>   12/26/2012  21:34         <DIR>    ..
>>    1/12/2013  16:27         <DIR>    bin
>>    1/14/2013  10:20         <DIR>    cgf
>>    ...
>> 
>> I don't have a foo directory but cmd was happy to just ignore that
>> fact and show my the root directory.  This is YA place where Windows
>> and Linux differ drastically.
>
>Indeed.  Before writing my mail I tested the "GetFullPathName" function,
>and I was not exactly surprised to find that it behaves as you describe
>for CMD.

Right.  It's not just CMD.  A standard windows program will behave
similarly.

cgf

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 16:13         ` Corinna Vinschen
@ 2013-01-14 20:29           ` Stephan Mueller
  2013-01-14 21:37             ` Ryan Johnson
  2013-01-15 12:33           ` Shaddy Baddah
  1 sibling, 1 reply; 23+ messages in thread
From: Stephan Mueller @ 2013-01-14 20:29 UTC (permalink / raw)
  To: cygwin

On Jan 14 01:17, Christopher Faylor wrote:
" It is a bug.  It's not just "~".  Any nonexistent directory will
" work, like "foo/..".

Corinna wrote:
" And it's a bug which isn't easily fixed.  Since about the dawn of time,
" Cygwin's core path handling evaluates the path in a non-POSIX manner,
" mainly for performance reasons.

and:
" Cygwin would have to check
" the existence of every single path component from left to right to
" emulate the POSIX requirements.  

and:
" The first step of converting a POSIX path to a Windows path is to
" normalize the path.  "." and ".." components are simply dropped:
"
" "a/b/./c"  -> "a\b\c"
"  "a/b/../c" -> "a\c"
"
" Then the path prefix is replaced by the matching mount point.

and:
" Ultimately I would be glad to
" be able to come up with more correct code in Cygwin while not getting
" slower, of course.  But that's wishful thinking for now.

Perhaps (as you may well have already considered):

- replace the path prefix by the mount point first?  (this may be naïve
  on my part, but it's not clear to me that .. early in a path should be able
  to influence which mount point is substituted)
- test directory existence of the component preceding .. before collapsing
  (in the example above) b/.. to nothing.
- trust that for a/b3/b2/b/../../../c, the existence test of a/b3/b2/b
  before collapsing b/.. to nothing implies existence of b2 and b3 so no
  further tests are needed for 'runs' of .. components with enough
  components before them

Understood that this is still going to cause a slowdown because paths with
.. are not uncommon, but it would reduce the number of additional
existence checks from one-per-path-component to one-per-run-of-..,
which means none in the case of paths without .. in them.

stephan();


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 20:29           ` Stephan Mueller
@ 2013-01-14 21:37             ` Ryan Johnson
  2013-01-15  8:44               ` Corinna Vinschen
  0 siblings, 1 reply; 23+ messages in thread
From: Ryan Johnson @ 2013-01-14 21:37 UTC (permalink / raw)
  To: cygwin

On 14/01/2013 3:24 PM, Stephan Mueller wrote:
> Perhaps (as you may well have already considered):
>
> - replace the path prefix by the mount point first?  (this may be naïve
>    on my part, but it's not clear to me that .. early in a path should be able
>    to influence which mount point is substituted)
If I mount something at /foo/bar/baz, then /foo/bar/baz/../../blah 
definitely shouldn't end up inside baz.

> - test directory existence of the component preceding .. before collapsing
>    (in the example above) b/.. to nothing.
> - trust that for a/b3/b2/b/../../../c, the existence test of a/b3/b2/b
>    before collapsing b/.. to nothing implies existence of b2 and b3 so no
>    further tests are needed for 'runs' of .. components with enough
>    components before them
>
> Understood that this is still going to cause a slowdown because paths with
> .. are not uncommon, but it would reduce the number of additional
> existence checks from one-per-path-component to one-per-run-of-..,
> which means none in the case of paths without .. in them.
The rest seems totally reasonable to me, FWIW.

I wouldn't put it past a Makefile (esp. one generated by autotools) or a 
gcc header search path to generate paths like: 
/a/b/c/d/../e/../f/../../../g/h (which resolves to a/g/h, if I counted 
dots correctly). Even then, though, it's "only" three checks to achieve 
posix-compliant behavior.

Ryan


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 12:23     ` Corinna Vinschen
  2013-01-14 14:37       ` Shaddy Baddah
  2013-01-14 15:27       ` Christopher Faylor
@ 2013-01-14 22:15       ` Thomas Wolff
  2013-01-15  8:56         ` Corinna Vinschen
  2013-01-15 19:50         ` Andrey Repin
  2 siblings, 2 replies; 23+ messages in thread
From: Thomas Wolff @ 2013-01-14 22:15 UTC (permalink / raw)
  To: cygwin

Am 14.01.2013 11:00, schrieb Corinna Vinschen:
> ...
>
> The first step of converting a POSIX path to a Windows path is to
> normalize the path.  "." and ".." components are simply dropped:
>
>    "a/b/./c"  -> "a\b\c"
>    "a/b/../c" -> "a\c"
which isn't correct already (even if everything exists) because if b is 
a symbolic link, "b/.." is *not* "." -
(I think I came across this bug a few times already without really 
noticing it as a bug, having taken it as some spurious glitch...)
(Not sure whether this case is covered by further arguments in this thread)
------
Thomas

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 21:37             ` Ryan Johnson
@ 2013-01-15  8:44               ` Corinna Vinschen
  0 siblings, 0 replies; 23+ messages in thread
From: Corinna Vinschen @ 2013-01-15  8:44 UTC (permalink / raw)
  To: cygwin

On Jan 14 16:37, Ryan Johnson wrote:
> On 14/01/2013 3:24 PM, Stephan Mueller wrote:
> >Perhaps (as you may well have already considered):
> >
> >- replace the path prefix by the mount point first?  (this may be naïve
> >   on my part, but it's not clear to me that .. early in a path should be able
> >   to influence which mount point is substituted)
> If I mount something at /foo/bar/baz, then /foo/bar/baz/../../blah
> definitely shouldn't end up inside baz.
> 
> >- test directory existence of the component preceding .. before collapsing
> >   (in the example above) b/.. to nothing.
> >- trust that for a/b3/b2/b/../../../c, the existence test of a/b3/b2/b
> >   before collapsing b/.. to nothing implies existence of b2 and b3 so no
> >   further tests are needed for 'runs' of .. components with enough
> >   components before them
> >
> >Understood that this is still going to cause a slowdown because paths with
> >.. are not uncommon, but it would reduce the number of additional
> >existence checks from one-per-path-component to one-per-run-of-..,
> >which means none in the case of paths without .. in them.
> The rest seems totally reasonable to me, FWIW.

That's a chicken/egg-like situation.  To be able to convert the
POSIX path prefix (aka mount point) to the matching DOS path prefix,
the path has to be normalized.  But without being converted to a
DOS path, you can't check path components.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 22:15       ` Thomas Wolff
@ 2013-01-15  8:56         ` Corinna Vinschen
  2013-01-15 19:50         ` Andrey Repin
  1 sibling, 0 replies; 23+ messages in thread
From: Corinna Vinschen @ 2013-01-15  8:56 UTC (permalink / raw)
  To: cygwin

On Jan 14 23:14, Thomas Wolff wrote:
> Am 14.01.2013 11:00, schrieb Corinna Vinschen:
> >...
> >
> >The first step of converting a POSIX path to a Windows path is to
> >normalize the path.  "." and ".." components are simply dropped:
> >
> >   "a/b/./c"  -> "a\b\c"
> >   "a/b/../c" -> "a\c"
> which isn't correct already (even if everything exists) because if b
> is a symbolic link, "b/.." is *not* "." -
> (I think I came across this bug a few times already without really
> noticing it as a bug, having taken it as some spurious glitch...)

Yes, that's a known downside.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 16:13         ` Corinna Vinschen
  2013-01-14 20:29           ` Stephan Mueller
@ 2013-01-15 12:33           ` Shaddy Baddah
  2013-02-07  7:00             ` Shaddy Baddah
  1 sibling, 1 reply; 23+ messages in thread
From: Shaddy Baddah @ 2013-01-15 12:33 UTC (permalink / raw)
  To: cygwin

Hi,

On 15 Jan 2013 03:13, Corinna Vinschen wrote:
>> It seems to me then that a patch to bash may be in order? I can see how
>> the bash check is the right thing to do. It doesn't want the special
>> tilde expansion to mask and disallow referencing of real tilde prefixed
>> paths. So the stat() check is the quick win to determine that.
>>
>>  From what I make of it, there needs to be a patch that, although can
>> work generically, adds checks only required for Cygwin. And therefore
>> is specific to the Cygwin package.
>>
>> The check would be an extension of the file_exists() function, perhaps
>> called tilde_file_exists(), which determines if the tilde prefix forms
>> a directory component of the path (strchr('/')?). If it does not, the
>> file_exists() check is sufficient. If it does, then the check of if
>> that directory exists is logically and'ed to the result of
>> file_exists().
>>
>> Does that sound about right?
>
> A check like this might be a good idea.  Ultimately I would be glad to
> be able to come up with more correct code in Cygwin while not getting
> slower, of course.  But that's wishful thinking for now.

Bash, patched in the way I have described, seems to fix the tab
completion issue.

I will tidy up the work and publish the patch at some point soon. I may
have taken a naive approach, so review comments are welcome.

-- 
Regards,
Shaddy

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-14 22:15       ` Thomas Wolff
  2013-01-15  8:56         ` Corinna Vinschen
@ 2013-01-15 19:50         ` Andrey Repin
  2013-01-15 20:03           ` Larry Hall (Cygwin)
  1 sibling, 1 reply; 23+ messages in thread
From: Andrey Repin @ 2013-01-15 19:50 UTC (permalink / raw)
  To: Thomas Wolff, cygwin

Greetings, Thomas Wolff!

>> The first step of converting a POSIX path to a Windows path is to
>> normalize the path.  "." and ".." components are simply dropped:
>>
>>    "a/b/./c"  -> "a\b\c"
>>    "a/b/../c" -> "a\c"
> which isn't correct already (even if everything exists) because if b is 
> a symbolic link, "b/.." is *not* "." -
> (I think I came across this bug a few times already without really 
> noticing it as a bug, having taken it as some spurious glitch...)
> (Not sure whether this case is covered by further arguments in this thread)

Only if it's a Cygwin symlink.
Which I'm avoiding in my daily work, since NTFS now offers the same
functionality.


--
WBR,
Andrey Repin (anrdaemon@freemail.ru) 15.01.2013, <23:38>

Sorry for my terrible english...


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-15 19:50         ` Andrey Repin
@ 2013-01-15 20:03           ` Larry Hall (Cygwin)
  0 siblings, 0 replies; 23+ messages in thread
From: Larry Hall (Cygwin) @ 2013-01-15 20:03 UTC (permalink / raw)
  To: cygwin

On 1/15/2013 2:39 PM, Andrey Repin wrote:
> Greetings, Thomas Wolff!
>
>>> The first step of converting a POSIX path to a Windows path is to
>>> normalize the path.  "." and ".." components are simply dropped:
>>>
>>>     "a/b/./c"  -> "a\b\c"
>>>     "a/b/../c" -> "a\c"
>> which isn't correct already (even if everything exists) because if b is
>> a symbolic link, "b/.." is *not* "." -
>> (I think I came across this bug a few times already without really
>> noticing it as a bug, having taken it as some spurious glitch...)
>> (Not sure whether this case is covered by further arguments in this thread)
>
> Only if it's a Cygwin symlink.
> Which I'm avoiding in my daily work, since NTFS now offers the same
> functionality.

Certainly if the native facilities work for you, you should use them.  But
I think there's been enough discussion in the past on this subject to
acknowledge that the native functionality doesn't support all that Cygwin
symlinks do.  I'm making this (very) brief statement for the benefit of
those that come across this in the archives.  Anyone seeking clarification
or more details should look in the archives for previous discussions on
this subject.

-- 
Larry

_____________________________________________________________________

A: Yes.
 > Q: Are you sure?
 >> A: Because it reverses the logical flow of conversation.
 >>> Q: Why is top posting annoying in email?

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-01-15 12:33           ` Shaddy Baddah
@ 2013-02-07  7:00             ` Shaddy Baddah
  2013-02-07 15:32               ` Eric Blake
  0 siblings, 1 reply; 23+ messages in thread
From: Shaddy Baddah @ 2013-02-07  7:00 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

Hi,

On 15 Jan 2013 23:33, Shaddy Baddah wrote:
<snip>
>>> From what I make of it, there needs to be a patch that, although can
>>> work generically, adds checks only required for Cygwin. And therefore
>>> is specific to the Cygwin package.
>>>
>>> The check would be an extension of the file_exists() function, perhaps
>>> called tilde_file_exists(), which determines if the tilde prefix forms
>>> a directory component of the path (strchr('/')?). If it does not, the
>>> file_exists() check is sufficient. If it does, then the check of if
>>> that directory exists is logically and'ed to the result of
>>> file_exists().
>>>
>>> Does that sound about right?
>>
>> A check like this might be a good idea. Ultimately I would be glad to
>> be able to come up with more correct code in Cygwin while not getting
>> slower, of course. But that's wishful thinking for now.
>
> Bash, patched in the way I have described, seems to fix the tab
> completion issue.
>
> I will tidy up the work and publish the patch at some point soon. I may
> have taken a naive approach, so review comments are welcome.

Please find the patch discussed attached. It probably needs to be a bit
more generic, maybe with some precompiler directives to limit it to
cygwin? Although if it is just for cygwin, perhaps it can just go in the
cygports patch?

Do I need to put an "attn bash maintainer" on the subject line?

[-- Attachment #2: cygwin-181692-bash-tilde-fix.diff --]
[-- Type: text/x-patch, Size: 1561 bytes --]

diff --git a/bashline.c b/bashline.c
index 3cbb18f..31841fa 100644
--- a/bashline.c
+++ b/bashline.c
@@ -3478,8 +3478,9 @@ quote_word_break_chars (text)
       if (mbschr (rl_completer_word_break_characters, *s))
 	*r++ = '\\';
       /* XXX -- check for standalone tildes here and backslash-quote them */
-      if (s == text && *s == '~' && file_exists (text))
+      if (s == text && *s == '~' && tilde_file_exists (text)) {
         *r++ = '\\';
+      }
       *r++ = *s;
     }
   *r = '\0';
diff --git a/general.c b/general.c
index fdadf1d..b279cbe 100644
--- a/general.c
+++ b/general.c
@@ -544,6 +544,28 @@ file_exists (fn)
 }
 
 int
+tilde_file_exists (fn)
+     char *fn;
+{
+  struct stat sb;
+  char *slash, *dirpart;
+  int istildedir;
+
+  istildedir = 0;
+
+  slash = strchr (fn, '/');
+  if ((slash != 0) && (slash != fn))
+    {
+      dirpart = (char *)xmalloc ((slash - fn) + 1);
+      strncpy (dirpart, fn, (slash - fn) + 1);
+      istildedir = file_isdir (dirpart);
+      free (dirpart);
+    }
+
+  return (!((slash != 0) && (slash != fn) && ! istildedir) && (stat (fn, &sb) == 0));
+}
+
+int
 file_isdir (fn)
      char *fn;
 {
diff --git a/general.h b/general.h
index 2b31c58..f6b7ab4 100644
--- a/general.h
+++ b/general.h
@@ -302,6 +302,7 @@ extern int sh_openpipe __P((int *));
 extern int sh_closepipe __P((int *));
 
 extern int file_exists __P((char *));
+extern int tilde_file_exists __P((char *));
 extern int file_isdir __P((char  *));
 extern int file_iswdir __P((char  *));
 extern int dot_or_dotdot __P((const char *));


[-- Attachment #3: Type: text/plain, Size: 218 bytes --]

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-02-07  7:00             ` Shaddy Baddah
@ 2013-02-07 15:32               ` Eric Blake
  2013-02-07 16:10                 ` Thomas Wolff
  0 siblings, 1 reply; 23+ messages in thread
From: Eric Blake @ 2013-02-07 15:32 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 2291 bytes --]

On 02/07/2013 12:00 AM, Shaddy Baddah wrote:
> Please find the patch discussed attached. It probably needs to be a bit
> more generic, maybe with some precompiler directives to limit it to
> cygwin? Although if it is just for cygwin, perhaps it can just go in the
> cygports patch?

The build of bash has nothing to do with cygports; if I decide to take
this, I will include something like it the next time I build bash for
cygwin.

> 
> Do I need to put an "attn bash maintainer" on the subject line?

Nah, I read the list.

>  	*r++ = '\\';
>        /* XXX -- check for standalone tildes here and backslash-quote them */
> -      if (s == text && *s == '~' && file_exists (text))
> +      if (s == text && *s == '~' && tilde_file_exists (text)) {

No need to add the {} here.

>          *r++ = '\\';
> +      }
>        *r++ = *s;
>      }
>    *r = '\0';
> diff --git a/general.c b/general.c
> index fdadf1d..b279cbe 100644
> --- a/general.c
> +++ b/general.c
> @@ -544,6 +544,28 @@ file_exists (fn)
>  }
>  
>  int
> +tilde_file_exists (fn)

This function is misnamed for what it does, which is attempt work around
the fact that cygwin's handling of .. is not POSIX-compliant.  I think a
better fix would be to change file_exists() itself instead of adding a
misnamed wrapper function; then bashline.c wouldn't even need patching.
 The string 'tilde' need not even be in the patch; what you are really
after is a function that says that if '..' is found within a string
being probed for existence, then add an additional check to see if the
prefix of that string exists as a directory.

But I don't mind experimenting with the idea - it remains to be seen
whether people will complain that bash is noticeably slower because it
takes time to double-check instead of rely on cygwin's non-POSIX
shortcut.  And the slowdown would only be on paths containing a '..'; I
would NOT be checking for symlinks (even though symlinks containing ..
are also being interpreted in a non-POSIX manner, it is much more
expensive to second-guess if you have to check every name for being a
symlink than it is to just check for literal ..).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-02-07 15:32               ` Eric Blake
@ 2013-02-07 16:10                 ` Thomas Wolff
  2013-02-07 16:24                   ` Corinna Vinschen
  0 siblings, 1 reply; 23+ messages in thread
From: Thomas Wolff @ 2013-02-07 16:10 UTC (permalink / raw)
  To: cygwin

Am 07.02.2013 16:30, schrieb Eric Blake:
> ...
>
> ...
> the fact that cygwin's handling of .. is not POSIX-compliant.  I think a
> better fix would be to change file_exists() itself instead of adding a
> misnamed wrapper function; then bashline.c wouldn't even need patching.
>   The string 'tilde' need not even be in the patch; what you are really
> after is a function that says that if '..' is found within a string
> being probed for existence, then add an additional check to see if the
> prefix of that string exists as a directory.
>
> But I don't mind experimenting with the idea - it remains to be seen
> whether people will complain that bash is noticeably slower because it
> takes time to double-check instead of rely on cygwin's non-POSIX
> shortcut.  And the slowdown would only be on paths containing a '..'; I
> would NOT be checking for symlinks (even though symlinks containing ..
> are also being interpreted in a non-POSIX manner, it is much more
> expensive to second-guess if you have to check every name for being a
> symlink than it is to just check for literal ..).
Do I interpret correctly that you talk about bash filename completion here?
Referring to http://cygwin.com/ml/cygwin/2013-01/msg00201.html,
I'd like to point out that while this ".." thing is a cygwin bug, or 
known downside as Corinna says,
the same issue occurs on Linux precisely with filename completion which 
isn't consistent there either.
So it would be "over-fixing" to handle that specifically in bash.

On the other hand, considering again this "downside":
If the core cygwin filesystem function would follow this approach,
simply checking for an occurrence of ".." first before resolving the 
filename,
wouldn't that be an acceptable fix without inappropriate penalty?

------
Thomas

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-02-07 16:10                 ` Thomas Wolff
@ 2013-02-07 16:24                   ` Corinna Vinschen
  2013-02-07 16:26                     ` Christopher Faylor
  0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2013-02-07 16:24 UTC (permalink / raw)
  To: cygwin

On Feb  7 17:10, Thomas Wolff wrote:
> Am 07.02.2013 16:30, schrieb Eric Blake:
> >...
> >
> >...
> >the fact that cygwin's handling of .. is not POSIX-compliant.  I think a
> >better fix would be to change file_exists() itself instead of adding a
> >misnamed wrapper function; then bashline.c wouldn't even need patching.
> >  The string 'tilde' need not even be in the patch; what you are really
> >after is a function that says that if '..' is found within a string
> >being probed for existence, then add an additional check to see if the
> >prefix of that string exists as a directory.
> >
> >But I don't mind experimenting with the idea - it remains to be seen
> >whether people will complain that bash is noticeably slower because it
> >takes time to double-check instead of rely on cygwin's non-POSIX
> >shortcut.  And the slowdown would only be on paths containing a '..'; I
> >would NOT be checking for symlinks (even though symlinks containing ..
> >are also being interpreted in a non-POSIX manner, it is much more
> >expensive to second-guess if you have to check every name for being a
> >symlink than it is to just check for literal ..).
> Do I interpret correctly that you talk about bash filename completion here?
> Referring to http://cygwin.com/ml/cygwin/2013-01/msg00201.html,
> I'd like to point out that while this ".." thing is a cygwin bug, or
> known downside as Corinna says,
> the same issue occurs on Linux precisely with filename completion
> which isn't consistent there either.
> So it would be "over-fixing" to handle that specifically in bash.
> 
> On the other hand, considering again this "downside":
> If the core cygwin filesystem function would follow this approach,
> simply checking for an occurrence of ".." first before resolving the
> filename,
> wouldn't that be an acceptable fix without inappropriate penalty?

You don't know what you're asking for (can of worms, etc.)

This is some kind of chicken egg problem.

- The path must be normalized, otherwise we can't reliably convert
  the POSIX path prefix to a Windows pathname.

- Only after converting to a Windows path, we can perform file checks.

- Rinse and repeat.

Also, the path normalization is performed in an entirely distinct
function from the mount point conversion, and this in turn is another
function than the path function handling symlinks and devices.
Changing that requires to implement the path conversion functionality
almost from scratch.  Given the age of some of these functions, I'd
like to have done that for a long time, but I'm constantly shying away
since I don't want to break what is working today.  There's, of course,
still the aforementioned chicken-egg problem.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: stat() and tilde prefix (was bad bash tab completion)
  2013-02-07 16:24                   ` Corinna Vinschen
@ 2013-02-07 16:26                     ` Christopher Faylor
  0 siblings, 0 replies; 23+ messages in thread
From: Christopher Faylor @ 2013-02-07 16:26 UTC (permalink / raw)
  To: cygwin

On Thu, Feb 07, 2013 at 05:23:22PM +0100, Corinna Vinschen wrote:
>On Feb  7 17:10, Thomas Wolff wrote:
>> Am 07.02.2013 16:30, schrieb Eric Blake:
>> >...
>> >
>> >...
>> >the fact that cygwin's handling of .. is not POSIX-compliant.  I think a
>> >better fix would be to change file_exists() itself instead of adding a
>> >misnamed wrapper function; then bashline.c wouldn't even need patching.
>> >  The string 'tilde' need not even be in the patch; what you are really
>> >after is a function that says that if '..' is found within a string
>> >being probed for existence, then add an additional check to see if the
>> >prefix of that string exists as a directory.
>> >
>> >But I don't mind experimenting with the idea - it remains to be seen
>> >whether people will complain that bash is noticeably slower because it
>> >takes time to double-check instead of rely on cygwin's non-POSIX
>> >shortcut.  And the slowdown would only be on paths containing a '..'; I
>> >would NOT be checking for symlinks (even though symlinks containing ..
>> >are also being interpreted in a non-POSIX manner, it is much more
>> >expensive to second-guess if you have to check every name for being a
>> >symlink than it is to just check for literal ..).
>> Do I interpret correctly that you talk about bash filename completion here?
>> Referring to http://cygwin.com/ml/cygwin/2013-01/msg00201.html,
>> I'd like to point out that while this ".." thing is a cygwin bug, or
>> known downside as Corinna says,
>> the same issue occurs on Linux precisely with filename completion
>> which isn't consistent there either.
>> So it would be "over-fixing" to handle that specifically in bash.
>> 
>> On the other hand, considering again this "downside":
>> If the core cygwin filesystem function would follow this approach,
>> simply checking for an occurrence of ".." first before resolving the
>> filename,
>> wouldn't that be an acceptable fix without inappropriate penalty?
>
>You don't know what you're asking for (can of worms, etc.)
>
>This is some kind of chicken egg problem.
>
>- The path must be normalized, otherwise we can't reliably convert
>  the POSIX path prefix to a Windows pathname.
>
>- Only after converting to a Windows path, we can perform file checks.
>
>- Rinse and repeat.
>
>Also, the path normalization is performed in an entirely distinct
>function from the mount point conversion, and this in turn is another
>function than the path function handling symlinks and devices.
>Changing that requires to implement the path conversion functionality
>almost from scratch.  Given the age of some of these functions, I'd
>like to have done that for a long time, but I'm constantly shying away
>since I don't want to break what is working today.  There's, of course,
>still the aforementioned chicken-egg problem.

What she said.

cgf

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2013-02-07 16:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-10 14:25 bad bash tab completion Shaddy Baddah
2012-08-10 16:54 ` Eric Blake
2013-01-14  5:21 ` stat() and tilde prefix (was bad bash tab completion) Shaddy Baddah
2013-01-14  6:17   ` Christopher Faylor
2013-01-14 12:23     ` Corinna Vinschen
2013-01-14 14:37       ` Shaddy Baddah
2013-01-14 16:13         ` Corinna Vinschen
2013-01-14 20:29           ` Stephan Mueller
2013-01-14 21:37             ` Ryan Johnson
2013-01-15  8:44               ` Corinna Vinschen
2013-01-15 12:33           ` Shaddy Baddah
2013-02-07  7:00             ` Shaddy Baddah
2013-02-07 15:32               ` Eric Blake
2013-02-07 16:10                 ` Thomas Wolff
2013-02-07 16:24                   ` Corinna Vinschen
2013-02-07 16:26                     ` Christopher Faylor
2013-01-14 15:27       ` Christopher Faylor
2013-01-14 16:05         ` Corinna Vinschen
2013-01-14 17:00           ` Christopher Faylor
2013-01-14 22:15       ` Thomas Wolff
2013-01-15  8:56         ` Corinna Vinschen
2013-01-15 19:50         ` Andrey Repin
2013-01-15 20:03           ` Larry Hall (Cygwin)

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 read-only IMAP folder(s) and NNTP newsgroup(s).