Re: [Patch]: mkdir -p and network drives
On Wed, May 18, 2005 at 02:42:12PM -0400, Pierre A. Humblet wrote:
>- Original Message -
>From: "Corinna Vinschen"
>To:
>Sent: Wednesday, May 18, 2005 12:48 PM
>Subject: Re: [Patch]: mkdir -p and network drives
>
>
>> Hi Pierre,
>>
>> I don't see a reason why you moved telldir just a few lines up.
>> Any reasoning, perhaps together with a ChangeLog entry?
>
>Nope, it was an accidental cut and I pasted it back a few lines off.
>
>>
>> Why did you remove fhandler_cygdrive::telldir but not
>> fhandler_cygdrive::seekdir? Both are just calling their base class
>> variants.
>
>I am still working on fhandler_cygdrive. I stopped to keep the size
>of the patch small.
>
>> > - else if (isvirtual_dev (dev.devn) && fileattr ==
>INVALID_FILE_ATTRIBUTES)
>> > -{
>> > - error = dev.devn == FH_NETDRIVE ? ENOSHARE : ENOENT;
>> > - return;
>> > -}
>>
>> I don't understand this one. What's the rational behind removing
>> these lines?
>
>- They won't work the day we support writing to the registry.
>- More generally, I think it's cleaner to do device specific error handling
>in the fhandlers, instead of adding conditionals in path.cc
>- In the case where one tries to create a file or directory on a virtual
>device,
>one gets EROFS with this patch, instead of ENOSHARE or ENOENT before.
>That seems more logical.
I checked in part of your patch last week and most of the rest today.
I don't agree that "EROFS" is more logical than "ENOSHARE" since
ENOSHARE is a more specific error message which provides more
information to the user. So, I have left path.cc intact.
Thanks for the patch.
cgf
Re: [Patch]: mkdir -p and network drives
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Wrong list. Redirecting. According to Vance Turner on 5/18/2005 11:06 PM: > I usually don't write you guys, I follow the thread to see how development > is going. > > Just a note. The ls command is't quite right. > > Ls -lRC wil not recursively list the files and directories in verbose mode. > The l flag seems to be ignored. Per POSIX, http://www.opengroup.org/onlinepubs/009695399/utilities/ls.html, -C and -l are mutually exclusive, and the last one specified wins. You are right that the -l flag is ignored in your example, but it is not a bug. - -- Life is short - so eat dessert first! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCjIVH84KuGfSFAYARAs53AJ9yVbtYsTOixPy09xEmMoJnA4InDQCg2XqZ UVT2dXJWEg5kLgrnZ22cqm4= =b5ty -END PGP SIGNATURE-
RE: [Patch]: mkdir -p and network drives
Original Message >From: Christopher Faylor >Sent: 19 May 2005 06:13 > On Wed, May 18, 2005 at 10:09:35PM -0700, Vance Turner wrote: >> Additional note >> >> ls -lRC - not working >> ls -RCl - working >> >> If you point out the source I will fix it. > > 1) This is not a bug reporting list. > > 2) This does not, as far as I can tell, have anything to do with the > subject of this message. You seem to be starting an unrelated thread. > > Please use the main cygwin mailing list for reporting bugs. > > cgf Also, "ls -C" and "ls -l" are mutually exclusive. Whichever one you place last on the command line wins. NAB. cheers, DaveK -- Can't think of a witty .sigline today
Re: [Patch]: mkdir -p and network drives
On Wed, May 18, 2005 at 10:09:35PM -0700, Vance Turner wrote: >Additional note > >ls -lRC - not working >ls -RCl - working > >If you point out the source I will fix it. 1) This is not a bug reporting list. 2) This does not, as far as I can tell, have anything to do with the subject of this message. You seem to be starting an unrelated thread. Please use the main cygwin mailing list for reporting bugs. cgf
RE: [Patch]: mkdir -p and network drives
Additional note ls -lRC - not working ls -RCl - working If you point out the source I will fix it.
RE: [Patch]: mkdir -p and network drives
I usually don't write you guys, I follow the thread to see how development
is going.
Just a note. The ls command is't quite right.
Ls -lRC wil not recursively list the files and directories in verbose mode.
The l flag seems to be ignored.
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Pierre A. Humblet
Sent: Wednesday, May 18, 2005 11:42 AM
To: cygwin-patches@cygwin.com
Subject: Re: [Patch]: mkdir -p and network drives
- Original Message -
From: "Corinna Vinschen"
To:
Sent: Wednesday, May 18, 2005 12:48 PM
Subject: Re: [Patch]: mkdir -p and network drives
> Hi Pierre,
>
> I don't see a reason why you moved telldir just a few lines up.
> Any reasoning, perhaps together with a ChangeLog entry?
Nope, it was an accidental cut and I pasted it back a few lines off.
>
> Why did you remove fhandler_cygdrive::telldir but not
> fhandler_cygdrive::seekdir? Both are just calling their base class
> variants.
I am still working on fhandler_cygdrive. I stopped to keep the size
of the patch small.
> > - else if (isvirtual_dev (dev.devn) && fileattr ==
INVALID_FILE_ATTRIBUTES)
> > -{
> > - error = dev.devn == FH_NETDRIVE ? ENOSHARE : ENOENT;
> > - return;
> > -}
>
> I don't understand this one. What's the rational behind removing
> these lines?
- They won't work the day we support writing to the registry.
- More generally, I think it's cleaner to do device specific error handling
in the fhandlers, instead of adding conditionals in path.cc
- In the case where one tries to create a file or directory on a virtual
device,
one gets EROFS with this patch, instead of ENOSHARE or ENOENT before.
That seems more logical.
Pierre
Re: [Patch]: mkdir -p and network drives
- Original Message -
From: "Corinna Vinschen"
To:
Sent: Wednesday, May 18, 2005 12:48 PM
Subject: Re: [Patch]: mkdir -p and network drives
> Hi Pierre,
>
> I don't see a reason why you moved telldir just a few lines up.
> Any reasoning, perhaps together with a ChangeLog entry?
Nope, it was an accidental cut and I pasted it back a few lines off.
>
> Why did you remove fhandler_cygdrive::telldir but not
> fhandler_cygdrive::seekdir? Both are just calling their base class
> variants.
I am still working on fhandler_cygdrive. I stopped to keep the size
of the patch small.
> > - else if (isvirtual_dev (dev.devn) && fileattr ==
INVALID_FILE_ATTRIBUTES)
> > -{
> > - error = dev.devn == FH_NETDRIVE ? ENOSHARE : ENOENT;
> > - return;
> > -}
>
> I don't understand this one. What's the rational behind removing
> these lines?
- They won't work the day we support writing to the registry.
- More generally, I think it's cleaner to do device specific error handling
in the fhandlers, instead of adding conditionals in path.cc
- In the case where one tries to create a file or directory on a virtual
device,
one gets EROFS with this patch, instead of ENOSHARE or ENOENT before.
That seems more logical.
Pierre
Re: [Patch]: mkdir -p and network drives
Hi Pierre,
On May 18 08:22, Pierre A. Humblet wrote:
> Here is the implementation of mkdir and rmdir with fhandlers.
Thanks for the patch. Chris is going to reply later today, but I have
some questions beforehand.
> +_off64_t
> +fhandler_disk_file::telldir (DIR *dir)
> +{
> + return dir->__d_position;
> +}
> +
> DIR *
> fhandler_disk_file::opendir ()
> {
> @@ -1268,12 +1376,6 @@ fhandler_disk_file::readdir (DIR *dir)
>return res;
> }
>
> -_off64_t
> -fhandler_disk_file::telldir (DIR *dir)
> -{
> - return dir->__d_position;
> -}
> -
I don't see a reason why you moved telldir just a few lines up.
Any reasoning, perhaps together with a ChangeLog entry?
> -_off64_t
> -fhandler_cygdrive::telldir (DIR *dir)
> -{
> - return fhandler_disk_file::telldir (dir);
> -}
> -
> void
> fhandler_cygdrive::seekdir (DIR *dir, _off64_t loc)
> {
Why did you remove fhandler_cygdrive::telldir but not
fhandler_cygdrive::seekdir? Both are just calling their base class
variants.
> - else if (isvirtual_dev (dev.devn) && fileattr == INVALID_FILE_ATTRIBUTES)
> -{
> - error = dev.devn == FH_NETDRIVE ? ENOSHARE : ENOENT;
> - return;
> -}
I don't understand this one. What's the rational behind removing
these lines?
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Project Co-Leader mailto:cygwin@cygwin.com
Red Hat, Inc.
Re: [Patch]: mkdir -p and network drives
Here is the implementation of mkdir and rmdir with fhandlers.
To prepare the day where proc_registry will allow writes,
I have removed setting PATH_RO and an error return from path.cc
(it's all handled in the fhandlers).
I have also removed obsolete code about fhandler_cygdrive.
There is another patch coming with minor corner case fixes.
Pierre
2005-05-18 Pierre Humblet <[EMAIL PROTECTED]>
* devices.h: Delete FH_CYGDRIVE_A and FH_CYGDRIVE_Z.
* fhandler.h (fhandler_base::mkdir): New virtual method.
(fhandler_base::rmdir): Ditto.
(fhandler_disk_file:mkdir): New method.
(fhandler_disk_file:rmdir): Ditto.
(fhandler_cygdrive::iscygdrive_root): Delete method.
(fhandler_cygdrive::telldir): Delete declaration.
* dir.cc (mkdir): Implement with fhandlers.
(rmdir): Ditto.
* fhandler.cc (fhandler_base::mkdir): New virtual method.
(fhandler_base::rmdir): Ditto.
* fhandler_disk_file.cc: Remove all uses of
fhandler_cygdrive::iscygdrive_root.
(fhandler_disk_file::mkdir): New method.
(fhandler_disk_file::rmdir): Ditto.
(fhandler_cygdrive::telldir): Delete.
* path.cc (path_conv::check): For virtual devices, do not set PATH_RO
and
do not set error in case of non-existence.Index: devices.h
===
RCS file: /cvs/src/src/winsup/cygwin/devices.h,v
retrieving revision 1.18
diff -u -p -r1.18 devices.h
--- devices.h 10 May 2005 20:56:06 - 1.18
+++ devices.h 18 May 2005 04:22:41 -
@@ -106,8 +106,6 @@ enum fh_devices
DEV_CYGDRIVE_MAJOR = 98,
FH_CYGDRIVE= FHDEV (DEV_CYGDRIVE_MAJOR, 0),
- FH_CYGDRIVE_A= FHDEV (DEV_CYGDRIVE_MAJOR, 'a'),
- FH_CYGDRIVE_Z= FHDEV (DEV_CYGDRIVE_MAJOR, 'z'),
DEV_TCP_MAJOR = 30,
FH_TCP = FHDEV (DEV_TCP_MAJOR, 36),
Index: fhandler.h
===
RCS file: /cvs/src/src/winsup/cygwin/fhandler.h,v
retrieving revision 1.247
diff -u -p -r1.247 fhandler.h
--- fhandler.h 17 May 2005 20:34:15 - 1.247
+++ fhandler.h 18 May 2005 04:22:42 -
@@ -346,6 +346,8 @@ class fhandler_base
void operator delete (void *);
virtual HANDLE get_guard () const {return NULL;}
virtual void set_eof () {}
+ virtual int mkdir (mode_t mode);
+ virtual int rmdir ();
virtual DIR *opendir ();
virtual dirent *readdir (DIR *);
virtual _off64_t telldir (DIR *);
@@ -664,6 +666,8 @@ class fhandler_disk_file: public fhandle
int msync (HANDLE h, caddr_t addr, size_t len, int flags);
bool fixup_mmap_after_fork (HANDLE h, DWORD access, int flags,
_off64_t offset, DWORD size, void *address);
+ int mkdir (mode_t mode);
+ int rmdir ();
DIR *opendir ();
struct dirent *readdir (DIR *);
_off64_t telldir (DIR *);
@@ -678,11 +682,9 @@ class fhandler_cygdrive: public fhandler
const char *pdrive;
void set_drives ();
public:
- bool iscygdrive_root () { return !dev ().minor; }
fhandler_cygdrive ();
DIR *opendir ();
struct dirent *readdir (DIR *);
- _off64_t telldir (DIR *);
void seekdir (DIR *, _off64_t);
void rewinddir (DIR *);
int closedir (DIR *);
Index: dir.cc
===
RCS file: /cvs/src/src/winsup/cygwin/dir.cc,v
retrieving revision 1.86
diff -u -p -r1.86 dir.cc
--- dir.cc 13 May 2005 15:46:05 - 1.86
+++ dir.cc 18 May 2005 04:22:42 -
@@ -221,39 +221,21 @@ extern "C" int
mkdir (const char *dir, mode_t mode)
{
int res = -1;
- SECURITY_ATTRIBUTES sa = sec_none_nih;
- security_descriptor sd;
+ fhandler_base *fh = NULL;
- path_conv real_dir (dir, PC_SYM_NOFOLLOW | PC_WRITABLE);
+ if (!(fh = build_fh_name (dir, NULL, PC_SYM_NOFOLLOW | PC_WRITABLE)))
+goto done; /* errno already set */;
- if (real_dir.error)
+ if (fh->error ())
{
- set_errno (real_dir.case_clash ? ECASECLASH : real_dir.error);
- goto done;
+ debug_printf ("got %d error from build_fh_name", fh->error ());
+ set_errno (fh->error ());
}
+ else if (!fh->mkdir (mode))
+res = 0;
+ delete fh;
- nofinalslash (real_dir.get_win32 (), real_dir.get_win32 ());
-
- if (allow_ntsec && real_dir.has_acls ())
-set_security_attribute (S_IFDIR | ((mode & 0) & ~cygheap->umask),
- &sa, sd);
-
- if (CreateDirectoryA (real_dir.get_win32 (), &sa))
-{
- if (!allow_ntsec && allow_ntea)
- set_file_attribute (false, NULL, real_dir.get_win32 (),
- S_IFDIR | ((mode & 0) & ~cygheap->umask));
-#ifdef HIDDEN_DOT_FILES
- char *c = strrchr (real_dir.get_win32 (), '\\');
- if ((c && c[1] == '.') || *real_dir.get_win32 () == '.')
- SetFileAttributes (real_dir.get_win32 (), FILE_ATTRIBUTE_HIDDEN);
-#endif
- res = 0;
-}
- else
-__seterrno ();
-
-done:
+ done:
syscall_printf ("%d = mkdir (%s, %d)",
Re: [Patch]: mkdir -p and network drives
On Fri, May 13, 2005 at 02:41:03PM +, Eric Blake wrote:
>> I added read-only filesystem checking to path_conv::check so the latest
>> snapshot seems to work fine with the latest coreutils (trixie is a
>> system in my home network which exports shares):
>
>Almost. With the 20050513 snapshot and coreutils-5.3.0-6, I am still getting:
>
>$ cd //eblake/share
>$ ls
>$ mkdir //eblake/share/dir
>$ ls
>dir share
>
>So you solved the mkdir("//server"), but not the mkdir("//server/share"), from
>creating a subdirectory in the most recent non-virtual current directory.
This is fixed in the latest snapshot. This turned out to be a really
simple fix which probably did not necessitate any of the EROFS stuff.
Oh well.
cgf
Re: [Patch]: mkdir -p and network drives
> I added read-only filesystem checking to path_conv::check so the latest
> snapshot seems to work fine with the latest coreutils (trixie is a
> system in my home network which exports shares):
Almost. With the 20050513 snapshot and coreutils-5.3.0-6, I am still getting:
$ cd //eblake/share
$ ls
$ mkdir //eblake/share/dir
$ ls
dir share
So you solved the mkdir("//server"), but not the mkdir("//server/share"), from
creating a subdirectory in the most recent non-virtual current directory.
> The "rmdir //" should really say "directory not empty" but that's a fix
> for another day.
Why change it? POSIX specifies EROFS if the directory to be removed is on a
read-only filesystem, which sounds like the right error for rmdir("//") to me.
--
Eric Blake
Re: [Patch]: mkdir -p and network drives
On Thu, May 12, 2005 at 04:02:22PM -0400, Christopher Faylor wrote: >On Thu, May 12, 2005 at 03:49:21PM -0400, Pierre A. Humblet wrote: >>- Original Message - >>From: "Corinna Vinschen" <[EMAIL PROTECTED]> >>To: >>Sent: Wednesday, May 11, 2005 4:53 AM >>Subject: Re: [Patch]: mkdir -p and network drives >> >> >>> I don't like the idea of isrofs being an inline function in dir.cc. >>> Wouldn't that be better a method in path_conv? It would be helpful >>> for other functions, too. For instance, unlink and symlink_worker. >>> In the (not so) long run we should really move all of these functions >>> into the fhandlers, though. >> >>After looking into it, moving mkdir and rmdir to fhandlers should be >>quite simple. I will do that early next week. > >In the meantime, I'm going to follow through on Corinna's suggestion. >Moving this into path.cc would mean that we could eventually add a "-r" >option to mount which would be a nice thing. > >I'm getting ready to roll out a 1.5.17 so I'll get something with EROFS >functionality into that. I added read-only filesystem checking to path_conv::check so the latest snapshot seems to work fine with the latest coreutils (trixie is a system in my home network which exports shares): bash-2.05b$ mkdir -p //trixie/share/tmp/foo/bar bash-2.05b$ ls -ld //trixie/share/tmp/foo/bar drwxr-xr-x 1 cgf None 0 May 13 09:47 //trixie/share/tmp/foo/bar bash-2.05b$ mkdir -p //trixie/blaz mkdir: cannot create directory `//trixie/blaz': Read-only file system bash-2.05b$ rmdir //trixie/share rmdir: `//trixie/share': Directory not empty bash-2.05b$ rmdir //trixie rmdir: `//trixie': Read-only file system bash-2.05b$ rmdir // rmdir: `//': Read-only file system bash-2.05b$ mkdir -p //foo/bar/blaz mkdir: cannot create directory `//foo': No such host or network path bash-2.05b$ rmdir //foo/bar rmdir: `//foo/bar': No such host or network path bash-2.05b$ rmdir //foo rmdir: `//foo': No such host or network path The support for read-only filesystems should be the same as in Pierre's patch, i.e., it is not comprehensive enough to be considered a full-fledged useful feature but it does seem to solve the mkdir -p problem. The "rmdir //" should really say "directory not empty" but that's a fix for another day. cgf
Re: [Patch]: mkdir -p and network drives
On Thu, May 12, 2005 at 03:49:21PM -0400, Pierre A. Humblet wrote: >- Original Message - >From: "Corinna Vinschen" <[EMAIL PROTECTED]> >To: >Sent: Wednesday, May 11, 2005 4:53 AM >Subject: Re: [Patch]: mkdir -p and network drives > > >> I don't like the idea of isrofs being an inline function in dir.cc. >> Wouldn't that be better a method in path_conv? It would be helpful >> for other functions, too. For instance, unlink and symlink_worker. >> In the (not so) long run we should really move all of these functions >> into the fhandlers, though. > >After looking into it, moving mkdir and rmdir to fhandlers should be >quite simple. I will do that early next week. In the meantime, I'm going to follow through on Corinna's suggestion. Moving this into path.cc would mean that we could eventually add a "-r" option to mount which would be a nice thing. I'm getting ready to roll out a 1.5.17 so I'll get something with EROFS functionality into that. cgf
Re: [Patch]: mkdir -p and network drives
- Original Message - From: "Corinna Vinschen" <[EMAIL PROTECTED]> To: Sent: Wednesday, May 11, 2005 4:53 AM Subject: Re: [Patch]: mkdir -p and network drives > I don't like the idea of isrofs being an inline function in dir.cc. > Wouldn't that be better a method in path_conv? It would be helpful > for other functions, too. For instance, unlink and symlink_worker. > In the (not so) long run we should really move all of these functions > into the fhandlers, though. After looking into it, moving mkdir and rmdir to fhandlers should be quite simple. I will do that early next week. Pierre
Re: [Patch]: mkdir -p and network drives
On May 10 20:53, Pierre A. Humblet wrote:
> * dir.cc (isrofs): New function.
> (mkdir): Check for FH_FS and use isrofs.
> (rmdir): Use isrofs.
>
> Index: dir.cc
> ===
> RCS file: /cvs/src/src/winsup/cygwin/dir.cc,v
> retrieving revision 1.84
> diff -u -p -r1.84 dir.cc
> --- dir.cc 16 Mar 2005 21:20:56 - 1.84
> +++ dir.cc 11 May 2005 00:38:11 -
> @@ -216,6 +216,13 @@ closedir (DIR *dir)
>return res;
> }
>
> +inline bool
> +isrofs(DWORD devn)
> +{
> + return devn == FH_PROC || devn == FH_REGISTRY
> +|| devn == FH_PROCESS || devn == FH_NETDRIVE;
> +}
> +
> /* mkdir: POSIX 5.4.1.1 */
> extern "C" int
> mkdir (const char *dir, mode_t mode)
I don't like the idea of isrofs being an inline function in dir.cc.
Wouldn't that be better a method in path_conv? It would be helpful
for other functions, too. For instance, unlink and symlink_worker.
In the (not so) long run we should really move all of these functions
into the fhandlers, though.
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Project Co-Leader mailto:cygwin@cygwin.com
Red Hat, Inc.
Re: [Patch]: mkdir -p and network drives
At 01:27 AM 5/11/2005 +, Eric Blake wrote:
>> At 11:11 AM 5/10/2005 -0400, Christopher Faylor wrote:
>>
>> So I restrained mkdir to only act on FH_FS.
>>
>> Ideally mkdir & rmdir should be part of the various handlers, but
>> there is no current payoff in doing so as directories can only be
>> created/deleted on FH_FS
>
>Would it ever be possible (or desirable) to let /proc/registry be writable
as a means of creating registry keys (mkdir) and values (creat)?
Yes IMO, and that day we will need to distribute {mk, rm}dir to the handlers.
There doesn't seem to be much demand for that however, people seem happy with
regtool.
Pierre
Re: [Patch]: mkdir -p and network drives
At 11:11 AM 5/10/2005 -0400, Christopher Faylor wrote:
>Could we see this as a unified-diff please?
Oops, but in retrospect it's a good thing. I did some more tests.
If c:\dev exists, then mkdir /dev/tty created c:\dev\tty (ditto
for the other /dev/xxx ), but rmdir /dev/tty would not delete
c:\dev\tty
mkdir /cygdrive created c:\cygwin\cygdrive
So I restrained mkdir to only act on FH_FS.
Ideally mkdir & rmdir should be part of the various handlers, but
there is no current payoff in doing so as directories can only be
created/deleted on FH_FS
>On Mon, May 09, 2005 at 08:16:36PM -0400, Pierre A. Humblet wrote:
>>At 06:19 PM 5/9/2005 +, Eric Blake wrote:
>>
>>>Second, the sequence chdir("//"), mkdir("machine") creates machine in the
>>>current directory.
>>
>>Old bug.
>>chdir("/proc"), mkdir("machine") produces the same result.
>>And mkdir("/proc"), mkdir("/proc/machine") creates c:\proc\machine
>>
>>The fix sets errno to EROFS, which is what rmdir is already doing.
>>Is that OK for coreutils?
Pierre
2005-05-11 Pierre Humblet <[EMAIL PROTECTED]>
* dir.cc (isrofs): New function.
(mkdir): Check for FH_FS and use isrofs.
(rmdir): Use isrofs.
Index: dir.cc
===
RCS file: /cvs/src/src/winsup/cygwin/dir.cc,v
retrieving revision 1.84
diff -u -p -r1.84 dir.cc
--- dir.cc 16 Mar 2005 21:20:56 - 1.84
+++ dir.cc 11 May 2005 00:38:11 -
@@ -216,6 +216,13 @@ closedir (DIR *dir)
return res;
}
+inline bool
+isrofs(DWORD devn)
+{
+ return devn == FH_PROC || devn == FH_REGISTRY
+|| devn == FH_PROCESS || devn == FH_NETDRIVE;
+}
+
/* mkdir: POSIX 5.4.1.1 */
extern "C" int
mkdir (const char *dir, mode_t mode)
@@ -231,6 +238,14 @@ mkdir (const char *dir, mode_t mode)
set_errno (real_dir.case_clash ? ECASECLASH : real_dir.error);
goto done;
}
+ if (real_dir.get_devn () != FH_FS)
+{
+ if (isrofs (real_dir.get_devn ()))
+ set_errno (EROFS);
+ else
+ set_errno (EEXIST);
+ goto done;
+}
nofinalslash (real_dir.get_win32 (), real_dir.get_win32 ());
@@ -263,14 +278,12 @@ extern "C" int
rmdir (const char *dir)
{
int res = -1;
- DWORD devn;
path_conv real_dir (dir, PC_SYM_NOFOLLOW | PC_FULL);
if (real_dir.error)
set_errno (real_dir.error);
- else if ((devn = real_dir.get_devn ()) == FH_PROC || devn == FH_REGISTRY
- || devn == FH_PROCESS)
+ else if (isrofs (real_dir.get_devn ()))
set_errno (EROFS);
else if (!real_dir.exists ())
set_errno (ENOENT);
Re: [Patch]: mkdir -p and network drives
On Mon, May 09, 2005 at 08:16:36PM -0400, Pierre A. Humblet wrote:
>At 06:19 PM 5/9/2005 +, Eric Blake wrote:
>
>>Second, the sequence chdir("//"), mkdir("machine") creates machine in the
>>current directory.
>
>Old bug.
>chdir("/proc"), mkdir("machine") produces the same result.
>And mkdir("/proc"), mkdir("/proc/machine") creates c:\proc\machine
>
>The fix sets errno to EROFS, which is what rmdir is already doing.
>Is that OK for coreutils?
>
>Pierre
>
>2005-05-10 Pierre Humblet <[EMAIL PROTECTED]>
>
> * dir.cc (isrofs): New function.
> (mkdir): Use isrofs.
> (rmdir): Ditto.
Could we see this as a unified-diff please?
cgf
Re: [Patch]: mkdir -p and network drives
At 06:19 PM 5/9/2005 +, Eric Blake wrote:
>Second, the sequence chdir("//"), mkdir("machine") creates machine in the
>current directory.
Old bug.
chdir("/proc"), mkdir("machine") produces the same result.
And mkdir("/proc"), mkdir("/proc/machine") creates c:\proc\machine
The fix sets errno to EROFS, which is what rmdir is already doing.
Is that OK for coreutils?
Pierre
2005-05-10 Pierre Humblet <[EMAIL PROTECTED]>
* dir.cc (isrofs): New function.
(mkdir): Use isrofs.
(rmdir): Ditto.
Index: dir.cc
===
RCS file: /cvs/src/src/winsup/cygwin/dir.cc,v
retrieving revision 1.84
diff -r1.84 dir.cc
218a219,225
> inline bool
> isrofs(DWORD devn)
> {
> return devn == FH_PROC || devn == FH_REGISTRY
> || devn == FH_PROCESS || devn == FH_NETDRIVE;
> }
>
233a241,245
> else if (isrofs (real_dir.get_devn ()))
> {
> set_errno (EROFS);
> goto done;
> }
266d277
< DWORD devn;
272,273c283
< else if ((devn = real_dir.get_devn ()) == FH_PROC || devn == FH_REGISTRY
< || devn == FH_PROCESS)
---
> else if (isrofs (real_dir.get_devn ()))
RE: [Patch]: mkdir -p and network drives
Original Message >From: Eric Blake >Sent: 06 May 2005 23:29 > Also, what should //.. resolve to, / or //? And if it resolves to /, > should // be an entry in the readdir() of /? I would argue that //.. > should resolve to //, meaning we just have two distinct roots in the > directory tree. Wouldn't it work to just have a magic directory called '/' in the root dir, just like the other magic dirs 'dev' and 'proc'? Then it would be called '//', and it could have '.' and '..' entries just like '/dev' and '/proc' do. Well, like /proc does, anyway. cheers, DaveK -- Can't think of a witty .sigline today
Re: [Patch]: mkdir -p and network drives
- Original Message -
From: "Eric Blake"
To:
Sent: Monday, May 09, 2005 2:19 PM
Subject: Re: [Patch]: mkdir -p and network drives
> Pierre A. Humblet phumblet.no-ip.org> writes:
> >
> > Here is a patch to allow mkdir -p to easily work with network
> > drives and to allow future enumeration of computers and of
> > network drives by ls -l.
> >
> > It works by defining a new FH_NETDRIVE virtual handler for
> > names such as // and //machine.
> > This also makes chdir work without additional change.
>
> I've just downloaded the 20050508 snapshot to play with this, and it still
> needs some work before coreutils-5.3.0-6 can be released. But it is an
> improvement!
>
> First, `ls -ld // //machine' show that these directories are mode 111
> (searchable, but not readable). Yet opendir("//") and
opendir("//machine")
> succeed, although POSIX requires that opendir(2) fail with EACCESS if the
> directory to be opened is not readable.
That's currently a feature. Being compliant means writing extra code
that will be junked when we make the directories readable.
In what way does non-compliance affect coreutils or the user?
A similar case is that getfacl reports the directories as r-x.
> Second, the sequence chdir("//"), mkdir("machine") creates machine in the
> current directory.
That's a bug. I will look into it. Thanks.
Pierre
Re: [Patch]: mkdir -p and network drives
Pierre A. Humblet phumblet.no-ip.org> writes:
>
> Here is a patch to allow mkdir -p to easily work with network
> drives and to allow future enumeration of computers and of
> network drives by ls -l.
>
> It works by defining a new FH_NETDRIVE virtual handler for
> names such as // and //machine.
> This also makes chdir work without additional change.
I've just downloaded the 20050508 snapshot to play with this, and it still
needs some work before coreutils-5.3.0-6 can be released. But it is an
improvement!
First, `ls -ld // //machine' show that these directories are mode 111
(searchable, but not readable). Yet opendir("//") and opendir("//machine")
succeed, although POSIX requires that opendir(2) fail with EACCESS if the
directory to be opened is not readable.
Second, the sequence chdir("//"), mkdir("machine") creates machine in the
current directory.
$ cd //eblake/share
$ ls
$ ~/coreutils-cvs/src/mkdir -p //eblake/share/dir
$ ls -F
dir/ eblake/ share/
A relevant portion of the strace is included below. Basically, mkdir
("//machine") (or chdir("//"), mkdir("machine")) needs to fail with EEXIST
(because it is always assumed that //machine already exists) or with EACCESS
(because there is no write access in //), rather than create a directory by
that name somewhere else.
69 4745479 [main] mkdir 10204 chdir: dir '//'
62 4745541 [main] mkdir 10204 normalize_posix_path: src //
66 4745607 [main] mkdir 10204 normalize_posix_path: // = normalize_posix_path
(//)
62 4745669 [main] mkdir 10204 mount_info::conv_to_win32_path: conv_to_win32_p
ath (//)
61 4745730 [main] mkdir 10204 set_flags: flags: binary (0x2)
74 4745804 [main] mkdir 10204 mount_info::conv_to_win32_path: src_path //, ds
t \\, flags 0x2, rc 0
77 4745881 [main] mkdir 10204 build_fh_pc: fh 0x61831710
67 4745948 [main] mkdir 10204 chdir: 0 = chdir() cygheap->cwd.posix '//' nati
ve '\\'
67 4746015 [main] mkdir 10204 normalize_posix_path: src eblake
61 4746076 [main] mkdir 10204 cwdstuff::get: posix //
60 4746136 [main] mkdir 10204 cwdstuff::get: (//) = cwdstuff::get (0x22EAC0,
260, 1, 0), errno 2
132 4746268 [main] mkdir 10204 normalize_posix_path: //eblake = normalize_posi
x_path (eblake)
67 4746335 [main] mkdir 10204 mount_info::conv_to_win32_path: conv_to_win32_p
ath (//eblake)
67 4746402 [main] mkdir 10204 set_flags: flags: binary (0x2)
61 4746463 [main] mkdir 10204 mount_info::conv_to_win32_path: src_path //ebla
ke, dst \\eblake, flags 0x2, rc 0
64 4746527 [main] mkdir 10204 build_fh_pc: fh 0x61831710
68 4746595 [main] mkdir 10204 cwdstuff::get: posix //
72 4746667 [main] mkdir 10204 cwdstuff::get: (\\) = cwdstuff::get (0x22E4E0,
260, 0, 0), errno 2
1561 4748228 [main] mkdir 10204 mkdir: 0 = mkdir (eblake, 511)
Re: [Patch]: mkdir -p and network drives
On Fri, May 06, 2005 at 04:29:38PM -0600, Eric Blake wrote: >-BEGIN PGP SIGNED MESSAGE- >Hash: SHA1 > >>I thought that Eric Blake implied that // *had* to be translated to /, >>as per POSIX. I wonder how many programs out there translate a >>standalone '//' to '/'. > >No, POSIX requires that / be untouched, // be implementation-defined >(hint - - define it on cygwin to stay untouched), and /// translate to >/. Ok. I knew that but I misinterpreted something you said as implying that '//' was not implementation defined. This was just a misreading on my part, though. cgf
Re: [Patch]: mkdir -p and network drives
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 > I thought that Eric Blake implied that // *had* to be translated to /, > as per POSIX. I wonder how many programs out there translate a > standalone '//' to '/'. No, POSIX requires that / be untouched, // be implementation-defined (hint - - define it on cygwin to stay untouched), and /// translate to /. - -- Life is short - so eat dessert first! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCe+/S84KuGfSFAYARAhb2AKCgqcwkbuXxzLrb2lbrfNk7va3ekgCgkl5U xCilioyN45W4FmvOYEc0Yno= =D3xl -END PGP SIGNATURE-
Re: [Patch]: mkdir -p and network drives
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
> Sigh. We need a bash maintainer.
> We need to have // working for mkdir -p to work, from what I
> understand of the code snippet that was sent to the list.
`mkdir -p' only uses chdir(), mkdir(), and stat() calls. For
//server/share/newdir, the strace sequence would be (with Paul Eggert's
patch):
chdir("//") - success
mkdir("server") - fail (hopefully EEXIST, but don't give up yet)
stat("server") - success (and S_ISDIR better be true)
chdir("server") - success
mkdir("share") - fail (again, hopefully with EEXIST)
stat("share") - success (and S_ISDIR is true)
chdir("share") - success
mkdir("newdir") - success
It is the stat calls where the decision is made whether the mkdir failed
because the directory was already there vs. couldn't be created, done in
that order to avoid races. I intend on releasing coreutils-5.3.0-6 as
soon as a snapshot has your minimal working //server as a directory, such
that the above strace works.
As to the accessibility of // and //server, your first cut should make
them mode 444 (dr--r--r--) for now. That way, you do not have to support
readdir() (which only makes sense if you have search permssion), but you
do support pathname resolution, and forbid adding or deleting servers or
shares as though they were files. If readdir() is added (and bash
patched, obviously), then change the accessibility to 555.
Also, what should //.. resolve to, / or //? And if it resolves to /,
should // be an entry in the readdir() of /? I would argue that //..
should resolve to //, meaning we just have two distinct roots in the
directory tree.
- --
Life is short - so eat dessert first!
Eric Blake [EMAIL PROTECTED]
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFCe++r84KuGfSFAYARAo7fAJ97YdifZyUBImxFNpTCOZ60UC+9wACgw76U
PSjw9LYS5eelqE1FdnKuU5Y=
=D774
-END PGP SIGNATURE-
Re: [Patch]: mkdir -p and network drives
On Fri, May 06, 2005 at 11:07:13AM -0400, Pierre A. Humblet wrote: >That's not Paul Eggert's position, >http://cygwin.com/ml/cygwin/2005-05/msg00251.html I don't expect >problems with //, we had it working in cvs for a while and only bash >had issues. Program translating // to / should already have problems >and they won't be affected if Cygwin keeps // Hmm. I missed that message. Seems rather... uncooperative... to check in a "fix" for Cygwin that wouldn't actually work on Cygwin. I certainly am enjoying the "Cygwin is broken so let's fix it!" implication, though. cgf
Re: [Patch]: mkdir -p and network drives
- Original Message - From: "Christopher Faylor" <[EMAIL PROTECTED]> To: Sent: Friday, May 06, 2005 10:58 AM Subject: Re: [Patch]: mkdir -p and network drives > On Fri, May 06, 2005 at 10:55:29AM -0400, Pierre A. Humblet wrote: > >- Original Message - > >From: "Christopher Faylor" <[EMAIL PROTECTED]> > >To: > >Sent: Friday, May 06, 2005 10:22 AM > >Subject: Re: [Patch]: mkdir -p and network drives > > > >>Well, that was kinda my point. If we can't remove the "//" handling > >>because it breaks bash then adding opendir/readdir stuff seems > >>premature except for the case of ls //foo which is entirely different > >>from ls //. > > > >Sigh. We need a bash maintainer. We need to have // working for mkdir > >-p to work, from what I understand of the code snippet that was sent to > >the list. > > I thought that Eric Blake implied that // *had* to be translated to /, > as per POSIX. I wonder how many programs out there translate a > standalone '//' to '/'. That's not Paul Eggert's position, http://cygwin.com/ml/cygwin/2005-05/msg00251.html I don't expect problems with //, we had it working in cvs for a while and only bash had issues. Program translating // to / should already have problems and they won't be affected if Cygwin keeps // Pierre
Re: [Patch]: mkdir -p and network drives
On Fri, May 06, 2005 at 10:55:29AM -0400, Pierre A. Humblet wrote: >- Original Message - >From: "Christopher Faylor" <[EMAIL PROTECTED]> >To: >Sent: Friday, May 06, 2005 10:22 AM >Subject: Re: [Patch]: mkdir -p and network drives > >>Well, that was kinda my point. If we can't remove the "//" handling >>because it breaks bash then adding opendir/readdir stuff seems >>premature except for the case of ls //foo which is entirely different >>from ls //. > >Sigh. We need a bash maintainer. We need to have // working for mkdir >-p to work, from what I understand of the code snippet that was sent to >the list. I thought that Eric Blake implied that // *had* to be translated to /, as per POSIX. I wonder how many programs out there translate a standalone '//' to '/'. cgf
Re: [Patch]: mkdir -p and network drives
- Original Message - From: "Christopher Faylor" <[EMAIL PROTECTED]> To: Sent: Friday, May 06, 2005 10:22 AM Subject: Re: [Patch]: mkdir -p and network drives > Well, that was kinda my point. If we can't remove the "//" handling because > it breaks bash then adding opendir/readdir stuff seems premature except for > the case of ls //foo which is entirely different from ls //. Sigh. We need a bash maintainer. We need to have // working for mkdir -p to work, from what I understand of the code snippet that was sent to the list. Pierre
Re: [Patch]: mkdir -p and network drives
On Fri, May 06, 2005 at 09:27:55AM -0400, Pierre A. Humblet wrote: >cgf wrote: >> On Thu, May 05, 2005 at 10:57:08PM -0400, Pierre A. Humblet wrote: > >>>The code should handle "//" correctly, but path.cc still transforms it >>>into "/", because of the bash bug. > >> Is that fixed in the current bash? > >AFAIK Corinna fixed it once, but the patch got lost and it's currently >not fixed. > >> So, I'd appreciate it if you would just move your fhandler_netdrive >> stuff to fhandler_netdrive.cc. > >Sure. Thanks for setting up the framework. > >> I didn't renumber FH_FS with above change. I wasn't sure why you did >> that. I don't think that there was a requirement that it has to be the >> lowest numbered minor device number. If there is a requirement like >> that we should change it. > >OK. No requirement, just aesthetic. There seemed to be a pattern. > >>>About implementing readdir: PTC... > >> I was thinking about doing this but how would it ever be invoked? > >With "ls -l //" or "ls -l //machine" > >> You can't do an opendir on "//", right? > >Sure you can (thanks to existing code in the virtual driver). Just remove >the >code in path.cc that changes "//" into "/". It's only/mainly there because >of bash. Well, that was kinda my point. If we can't remove the "//" handling because it breaks bash then adding opendir/readdir stuff seems premature except for the case of ls //foo which is entirely different from ls //. cgf
Re: [Patch]: mkdir -p and network drives
cgf wrote: > On Thu, May 05, 2005 at 10:57:08PM -0400, Pierre A. Humblet wrote: >>The code should handle "//" correctly, but path.cc still transforms it >>into "/", because of the bash bug. > Is that fixed in the current bash? AFAIK Corinna fixed it once, but the patch got lost and it's currently not fixed. > So, I'd appreciate it if you would just move your fhandler_netdrive > stuff to fhandler_netdrive.cc. Sure. Thanks for setting up the framework. > I didn't renumber FH_FS with above change. I wasn't sure why you did > that. I don't think that there was a requirement that it has to be the > lowest numbered minor device number. If there is a requirement like > that we should change it. OK. No requirement, just aesthetic. There seemed to be a pattern. >>About implementing readdir: PTC... > I was thinking about doing this but how would it ever be invoked? With "ls -l //" or "ls -l //machine" > You can't do an opendir on "//", right? Sure you can (thanks to existing code in the virtual driver). Just remove the code in path.cc that changes "//" into "/". It's only/mainly there because of bash. Pierre
Re: [Patch]: mkdir -p and network drives
On Thu, May 05, 2005 at 10:57:08PM -0400, Pierre A. Humblet wrote: > >Here is a patch to allow mkdir -p to easily work with network >drives and to allow future enumeration of computers and of >network drives by ls -l. > >It works by defining a new FH_NETDRIVE virtual handler for >names such as // and //machine. >This also makes chdir work without additional change. > >The code for the new handler is currently in fhandler_virtual.cc, for >simplicity (not an expert on Makefile and fomit-frame-pointer). >It should eventually be placed in fhandler_netdrive.cc Yes. >The code should handle "//" correctly, but path.cc still transforms it >into "/", because of the bash bug. Is that fixed in the current bash? >I have directly edited devices.cc instead of using the devices.in >magic. I think that the fact that we have to check for //foo/bar in mount_info::conv_to_win32_path indicates some kind of design flaw -- not one that you introduced, of course. I just don't think that this function should be getting paths that are known not to be associated with the mount table. I would have expected that a //foo/bar style path would not have made it down that far. I also notice that the cygdrive comment has been misplaced over time. Could you fix that when you check this in? I just checked in a dummy fhandler_netdrive.cc, added fhandler_netdrive.o to the Makefile, added FH_NETDRIVE to devices.h, defined dev_netdrive_storage in devices.in, and regenerated devices.in. So, I'd appreciate it if you would just move your fhandler_netdrive stuff to fhandler_netdrive.cc. I didn't renumber FH_FS with above change. I wasn't sure why you did that. I don't think that there was a requirement that it has to be the lowest numbered minor device number. If there is a requirement like that we should change it. Anyway, feel free to check this in. >About implementing readdir: PTC... I was thinking about doing this but how would it ever be invoked? You can't do an opendir on "//", right? cgf
