Re: Odd, is it not? mkdir 'e:\' cannot be undone by rmdir 'e:\' ...

[Ken Brown <kbrown@cornell.edu>]

On 9/1/2019 1:38 PM, Houder wrote:
> On Fri, 30 Aug 2019 11:54:27, Houder  wrote:

[...]

> As the directory "/foo" had been correctly created, I turned to
> path_conv::check(), which is called when build_fhname() creates
> the path_conv object (also called pc) -- see dtable.cc.
> 
> Examining this (obsure) method in path.cc, I corrected the code
> in 2 places:
> 
> ---
>        if (dev.isfs ())
>          {
>            //if (strncmp (path, "\\\\.\\", 4)) <==== 1171
>            if ( ! strncmp (path, "\\\\.\\", 4)) // <==== [1]
>              {
>                if (!tail || tail == path)
>                  /* nothing */;
>                else if (tail[-1] != '\\')
>                  *tail = '\0'; <==== Ah! (you should not do that!)
>                else
>                  {
>                    error = ENOENT;
>                    return;
>                  }
>              }
> 
> [1] this code should be executed only if path == '\\.\' !!

I don't agree with your analysis here.

First, the strncmp() call is testing whether path *starts with* '\\.\', not 
whether path == '\\.\'.  For example, path might be a UNC device name like 
'\\.\c:'.  Second, as the original code indicates (before your correction), we 
do *not* want to execute the code in that case, since we might be mutilating the 
device name or incorrectly setting ENOENT.

On the other hand, I agree that there's something wrong with that code snippet. 
Comparing tail with path [which is the class member this->path] makes no sense 
here, because tail is a pointer into path_copy.  So I think line 1173 should read

	      if (!tail || tail == path_copy)

If this condition fails, then it's legitimate to refer to tail[-1] two lines later.

Observe next that path_copy contains no backslashes, so I think line 1175 should 
probably be

	      else if (tail[-1] != '/')

I don't immediately see why we would then set *tail = '\0' in this case, because 
I think *tail is already 0 if we get here and tail[-1] != '/'.  But maybe I'm 
missing something.

I need to think about this further, but I wanted to write down my initial 
thoughts before your bug report gets forgotten.

To be continued.

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