public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
* Solved. Odd, is it not? mkdir 'e:\' cannot be undone by rmdir 'e:\' ...
@ 2019-09-06 21:53 Houder
  2019-09-21 22:45 ` Ken Brown
  2019-09-22 11:55 ` Houder
  0 siblings, 2 replies; 4+ messages in thread
From: Houder @ 2019-09-06 21:53 UTC (permalink / raw)
  To: cygwin

To those still interested! :-P

I expressed surprise that mkdir e:/ does NOT refer to the drive, but
rmdir e:/ does. Likewise do ls, stat ...

    https://cygwin.com/ml/cygwin/2019-08/msg00334.html
    ( Odd, is it not? mkdir 'e:\' cannot be undone by rmdir 'e:\' ... )

Why could mkdir not be made symmetrical to rmdir w/ regard to e:/ ?????

After glancing over
    path_conv::check() in winsup/cygwin/path.cc,

    https://cygwin.com/ml/cygwin/2019-08/msg00418.html
    ( Date: Fri, 30 Aug 2019 11:54:27 +0200 )

I decided to remove Eric B.'s code snippet (2009, 26th Sep) from
    mkdir() in winsup/cygwin/dir.cc,

i.e. I decided to delete the following lines:

      if (isdirsep (dir[strlen (dir) - 1]))
        {
          /* This converts // to /, but since both give EEXIST, we're okay.  */
          char *buf;
          char *p = stpcpy (buf = tp.c_get (), dir) - 1;
          dir = buf;
          while (p > dir && isdirsep (*p))
            *p-- = '\0';
        }

While I took a closer look at the source code, I found a BUG in
path_conv::check() in winsup/cygwin/path.cc

    https://cygwin.com/ml/cygwin/2019-08/msg00418.html
    ( Date: Sun, 01 Sep 2019 19:38:11 +0200 )

      if (dev.isfs ())
        {
          //if (strncmp (path, "\\\\.\\", 4)) <==== 1171
          if ( ! strncmp (path, "\\\\.\\", 4)) // <==== [1]
..
[1] this code should be executed only if path == '\\.\' !!

On September 3rd, I discovered that dropping Eric B.'s code snippet,
would introduce a BUG:

    https://cygwin.com/ml/cygwin/2019-09/msg00015.html
    ( Date: Tue, 03 Sep 2019 10:39:54 +0200 )

64-@@ ln -s aap noot
..
64-@@ rmdir aap
64-@@ mkdir noot
mkdir: cannot create directory ‘noot’: File exists
64-@@ mkdir noot/ <==== Whao! So that is what Eric indicated in his commit!
64-@@ ls -ld aap <==== WRONG! WRONG!
drwxr-xr-x+ 1 Henri None 0 Sep  3 10:28 aap

Different from Posix, Linux does not allow the creation of the directory
aap ...   (btw, neither should rmdir delete an existing directory aap if
noot/ is specified)

While waiting for a reaction by Eric Blake, I decided to take a closer
look at path_conv::check() ... Could a solution be found in this method?

(path arguments to (all?) commands are processed by this method)

Basically, this method consists of a 'double loop', as follows:

    for (;;) // outer loop
      for (;;) // inner loop

 - the inner loop tests whether or not a path component is a symlnk
 - if it is, the outer loop is reentered, where the symlnk part of
   the path is replaced by the target
 - finally, the algorithm bails out of both loops if a "real" path
   is found (or not)

Or something very near to this explanation ...

In case the last component is a symlnk, the name of the symlnk is
saved internally if the path had not been specified w/ a trailing
slash. Otherwise the name of the target is saved internally.

In short, there is a basic difference between specifying a path
w/ a trailing slash or not ...

I decided that modifying this method was not an option; I turned
my attention again to mkdir() in winsup/cygwin/dir.cc

To make mkdir behave as in Linux, and in case the last component
is a symlnk, the name of the symlnk must be saved internally, not
the name of the target.
This was achieved by stripping trailing slashes ...

The same patch should have been applied to rmdir ...

However an exception can be made for e:/ (or e:\), as follows:

--
      char flag = '\0';
      // strip trailing dirsep's, while remembering the last one
      if (isdirsep (dir[strlen (dir) - 1]))
        {
          flag = dir[strlen (dir) - 1];
          /* This converts // to /, but since both give EEXIST, we're okay.  */
          char *buf;
          char *p = stpcpy (buf = tp.c_get (), dir) - 1;
          dir = buf;
          while (p > dir && isdirsep (*p))
           {
            flag = *p;
            *p-- = '\0';
           }
        }

      // reattach dirsep in case of x: and flag != '\0'
      if ( (strlen (dir) == 2)
           && (dir[1] == ':')
           && isalpha (dir[0]) && flag != '\0' )
        {
          char *buf = tp.c_get ();
          buf[0] = dir[0];
          buf[1] = ':';
          buf[2] = flag;
          buf[3] = '\0';
          dir = buf;
        }
--

I applied the above patch to both mkdir() and rmdir() ...

Henri

-----
Tests:

3.1.0-0.2-patched => both mkdir and rmdir/ dir.cc have been
modified (plus bug removed in path.cc)

64-@@ ln -s aap noot

64-@@ ls -l noot
lrwxrwxrwx 1 Henri None 3 Sep  5 10:39 noot -> aap
64-@@ stat noot
  File: noot -> aap
  Size: 3               Blocks: 1          IO Block: 65536  symbolic link
Device: 33d91880h/869865600d    Inode: 23643898043927143  Links: 1
Access: (0777/lrwxrwxrwx)  Uid: ( 1000/   Henri)   Gid: (  513/    None)
Access: 2019-09-05 10:39:48.879895700 +0200
Modify: 2019-09-05 10:39:48.879895700 +0200
Change: 2019-09-05 10:39:48.879895700 +0200
 Birth: 2019-09-05 10:39:48.879895700 +0200

64-@@ ls -l noot/
ls: cannot access 'noot/': No such file or directory
64-@@ stat noot/
stat: cannot stat 'noot/': No such file or directory

64-@@ mkdir noot
mkdir: cannot create directory ‘noot’: File exists
64-@@ mkdir noot/
mkdir: cannot create directory ‘noot/’: File exists

64-@@ rmdir noot
rmdir: failed to remove 'noot': Not a directory
64-@@ rmdir noot/
rmdir: failed to remove 'noot/': Not a directory

--
64-@@ mkdir aap

64-@@ ls -l noot
lrwxrwxrwx 1 Henri None 3 Sep  5 10:39 noot -> aap
64-@@ stat noot
  File: noot -> aap
  Size: 3               Blocks: 1          IO Block: 65536  symbolic link
Device: 33d91880h/869865600d    Inode: 23643898043927143  Links: 1
Access: (0777/lrwxrwxrwx)  Uid: ( 1000/   Henri)   Gid: (  513/    None)
Access: 2019-09-05 10:39:48.879895700 +0200
Modify: 2019-09-05 10:39:48.879895700 +0200
Change: 2019-09-05 10:39:48.879895700 +0200
 Birth: 2019-09-05 10:39:48.879895700 +0200

64-@@ ls -l noot/
total 0
64-@@ stat noot/
  File: noot/
  Size: 0               Blocks: 0          IO Block: 65536  directory
Device: 33d91880h/869865600d    Inode: 10977524091947662  Links: 1
Access: (0755/drwxr-xr-x)  Uid: ( 1000/   Henri)   Gid: (  513/    None)
Access: 2019-09-05 10:41:13.759644800 +0200
Modify: 2019-09-05 10:41:13.759644800 +0200
Change: 2019-09-05 10:41:13.759644800 +0200
 Birth: 2019-09-05 10:41:13.759644800 +0200

64-@@ mkdir noot
mkdir: cannot create directory ‘noot’: File exists
64-@@ mkdir noot/
mkdir: cannot create directory ‘noot/’: File exists

64-@@ rmdir noot
rmdir: failed to remove 'noot': Not a directory
64-@@ rmdir noot/
rmdir: failed to remove 'noot/': Not a directory

=====


--
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] 4+ messages in thread

* Re: Solved. Odd, is it not? mkdir 'e:\' cannot be undone by rmdir 'e:\' ...
  2019-09-06 21:53 Solved. Odd, is it not? mkdir 'e:\' cannot be undone by rmdir 'e:\' Houder
@ 2019-09-21 22:45 ` Ken Brown
  2019-09-22  9:47   ` Houder
  2019-09-22 11:55 ` Houder
  1 sibling, 1 reply; 4+ messages in thread
From: Ken Brown @ 2019-09-21 22:45 UTC (permalink / raw)
  To: cygwin

On 9/6/2019 5:53 PM, Houder wrote:
> However an exception can be made for e:/ (or e:\), as follows:
> 
> --
>        char flag = '\0';
>        // strip trailing dirsep's, while remembering the last one
>        if (isdirsep (dir[strlen (dir) - 1]))
>          {
>            flag = dir[strlen (dir) - 1];
>            /* This converts // to /, but since both give EEXIST, we're okay.  */
>            char *buf;
>            char *p = stpcpy (buf = tp.c_get (), dir) - 1;
>            dir = buf;
>            while (p > dir && isdirsep (*p))
>             {
>              flag = *p;
>              *p-- = '\0';
>             }
>          }
> 
>        // reattach dirsep in case of x: and flag != '\0'
>        if ( (strlen (dir) == 2)
>             && (dir[1] == ':')
>             && isalpha (dir[0]) && flag != '\0' )
>          {
>            char *buf = tp.c_get ();
>            buf[0] = dir[0];
>            buf[1] = ':';
>            buf[2] = flag;
>            buf[3] = '\0';
>            dir = buf;
>          }

I think you can simplify this by eliminating the second part and changing the 
first part to the following:

         char sep = dir[strlen (dir) - 1];
         if (isdirsep (sep)
           {
             /* This converts // to /, but since both give EEXIST, we're okay. */
             char *buf;
             char *p = stpcpy (buf = tp.c_get (), dir) - 1;
             dir = buf;
             while (p > dir && isdirsep (*p))
               *p-- = '\0';
             /* Reattach dirsep in case of "x:". */
             if (p == dir + 1 && *p == ':' && isalpha (dir[0]))
               p[1] = sep;
           }

Ken

--
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] 4+ messages in thread

* Re: Solved. Odd, is it not? mkdir 'e:\' cannot be undone by rmdir 'e:\' ...
  2019-09-21 22:45 ` Ken Brown
@ 2019-09-22  9:47   ` Houder
  0 siblings, 0 replies; 4+ messages in thread
From: Houder @ 2019-09-22  9:47 UTC (permalink / raw)
  To: cygwin

On Sat, 21 Sep 2019 21:02:37, Ken Brown  wrote:
[snip]

> I think you can simplify this by eliminating the second part and changing
> the first part to the following:
> 
>          char sep = dir[strlen (dir) - 1];
>          if (isdirsep (sep)
>            {
>              /* This converts // to /, but since both give EEXIST, we're okay. */
>              char *buf;
>              char *p = stpcpy (buf = tp.c_get (), dir) - 1;
>              dir = buf;
>              while (p > dir && isdirsep (*p))
>                *p-- = '\0';
>              /* Reattach dirsep in case of "x:". */
>              if (p == dir + 1 && *p == ':' && isalpha (dir[0]))
>                p[1] = sep;
>            }

Hi Ken,

The ball is your court now. Having said that, in case of multiple trailing
dirseps, your code will reattach the last one.

My code will reattach the first dirsep, not the last one.

And dirseps can be both \ and /.

But again, the ball is your court now ...

Regards,
Henri

=====


--
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] 4+ messages in thread

* Re: Solved. Odd, is it not? mkdir 'e:\' cannot be undone by rmdir 'e:\' ...
  2019-09-06 21:53 Solved. Odd, is it not? mkdir 'e:\' cannot be undone by rmdir 'e:\' Houder
  2019-09-21 22:45 ` Ken Brown
@ 2019-09-22 11:55 ` Houder
  1 sibling, 0 replies; 4+ messages in thread
From: Houder @ 2019-09-22 11:55 UTC (permalink / raw)
  To: cygwin

Nothing new here; only correction of mistakes that I made (I decided
to review my e-mail because Ken Brwon took an interrest in the subject
matter).

On Fri, 06 Sep 2019 23:53:05, Houder  wrote:
> To those still interested! :-P
[snip]

> While I took a closer look at the source code, I found a BUG in
> path_conv::check() in winsup/cygwin/path.cc
> 
>     https://cygwin.com/ml/cygwin/2019-08/msg00418.html <==== wrong
>     ( Date: Sun, 01 Sep 2019 19:38:11 +0200 )

Correction:
https://cygwin.com/ml/cygwin/2019-09/msg00001.html

[snip]

> On September 3rd, I discovered that dropping Eric B.'s code snippet,
> would introduce a BUG:
> 
>     https://cygwin.com/ml/cygwin/2019-09/msg00015.html
>     ( Date: Tue, 03 Sep 2019 10:39:54 +0200 )
> 
> 64-@@ ln -s aap noot
> ..
> 64-@@ rmdir aap
> 64-@@ mkdir noot
> mkdir: cannot create directory ‘noot’: File exists
> 64-@@ mkdir noot/ <==== Whao! So that is what Eric indicated in his commit!
> 64-@@ ls -ld aap <==== WRONG! WRONG!
> drwxr-xr-x+ 1 Henri None 0 Sep  3 10:28 aap
> 
> Different from Posix, Linux does not allow the creation of the directory
> aap ...   (btw, neither should rmdir delete an existing directory aap if
> noot/ is specified)

Correction:
Linux is in agreement w/ Posix.
Cygwin is NOT in agreement w/ Posix (and Linux)i wrt to rmdir(2).

> While waiting for a reaction by Eric Blake, I decided to take a closer
> look at path_conv::check() ... Could a solution be found in this method?
> 
> (path arguments to (all?) commands are processed by this method)
> 
> Basically, this method consists of a 'double loop', as follows:
> 
>     for (;;) // outer loop
>       for (;;) // inner loop
> 
>  - the inner loop tests whether or not a path component is a symlnk
>  - if it is, the outer loop is reentered, where the symlnk part of
>    the path is replaced by the target
>  - finally, the algorithm bails out of both loops if a "real" path
>    is found (or not)
> 
> Or something very near to this explanation ...
> 
> In case the last component is a symlnk, the name of the symlnk is
> saved internally if the path had not been specified w/ a trailing
> slash. Otherwise the name of the target is saved internally.

Correction:
A symlnk is always followed if the pathname ends w/ a trailing slash;
if not, it depends on what the system call specified when it invoked
"path resolution" (path_conv::check() ).
If the system call specified "do not follow", "path resolution" does
not follow the symlnk (again, if path does NOT end w/ a trailing /).

> In short, there is a basic difference between specifying a path
> w/ a trailing slash or not ...

Correct! Look at how the response is different between stat final
and stat final/ in case of a symlnk.
(stat(1) basically calls lstat(2), which directs path resolution
 NOT to follow a symlnk; however that directive is ignored by path
 resolution if the pathname ends w/ a slash)

mkdir(2) and rmdir(2) are exceptions, in that these syscalls must
strip trailing slashes; they must also specify "do not follow".

The reason is, that these syscalls must not accept a symlnk as an
argument.

Henri

=====


--
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] 4+ messages in thread

end of thread, other threads:[~2019-09-22  9:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06 21:53 Solved. Odd, is it not? mkdir 'e:\' cannot be undone by rmdir 'e:\' Houder
2019-09-21 22:45 ` Ken Brown
2019-09-22  9:47   ` Houder
2019-09-22 11:55 ` Houder

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).