[PATCH 0/4] Fix rename bug with socket files

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/4] Fix rename bug with socket files

Ken Brown-6
The last patch of this series fixes the bug reported here:

  https://cygwin.com/ml/cygwin/2019-07/msg00139.html

The first three patches do some cleanup of the is**device() methods.

Ken

Ken Brown (4):
  Cygwin: fhandler_*: remove isdevice() and is_auto_device()
  Cygwin: remove path_conv::is_auto_device()
  Cygwin: remove path_conv::is_fs_device()
  Cygwin: socket files are not lnk special files

 winsup/cygwin/fhandler.h            | 3 ---
 winsup/cygwin/fhandler_disk_file.cc | 4 ++--
 winsup/cygwin/fhandler_raw.cc       | 2 +-
 winsup/cygwin/path.cc               | 2 +-
 winsup/cygwin/path.h                | 7 ++++---
 5 files changed, 8 insertions(+), 10 deletions(-)

--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/4] Cygwin: fhandler_*: remove isdevice() and is_auto_device()

Ken Brown-6
isdevice() is used only in the definition of is_auto_device().  And
the latter is used only once, in a context where isdevice() always
returns true.
---
 winsup/cygwin/fhandler.h      | 3 ---
 winsup/cygwin/fhandler_raw.cc | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 0da87e985..e0a8d4101 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -414,7 +414,6 @@ public:
   virtual bool is_tty () const { return false; }
   virtual bool ispipe () const { return false; }
   virtual pid_t get_popen_pid () const {return 0;}
-  virtual bool isdevice () const { return true; }
   virtual bool isfifo () const { return false; }
   virtual int ptsname_r (char *, size_t);
   virtual class fhandler_socket *is_socket () { return NULL; }
@@ -459,7 +458,6 @@ public:
   virtual void seekdir (DIR *, long);
   virtual void rewinddir (DIR *);
   virtual int closedir (DIR *);
-  bool is_auto_device () {return isdevice () && !dev ().isfs ();}
   bool is_fs_special () {return pc.is_fs_special ();}
   bool issymlink () {return pc.issymlink ();}
   bool __reg2 device_access_denied (int);
@@ -1455,7 +1453,6 @@ class fhandler_disk_file: public fhandler_base
   int dup (fhandler_base *child, int);
   void fixup_after_fork (HANDLE parent);
   int mand_lock (int, struct flock *);
-  bool isdevice () const { return false; }
   int __reg2 fstat (struct stat *buf);
   int __reg1 fchmod (mode_t mode);
   int __reg2 fchown (uid_t uid, gid_t gid);
diff --git a/winsup/cygwin/fhandler_raw.cc b/winsup/cygwin/fhandler_raw.cc
index bd47b6010..7c341d895 100644
--- a/winsup/cygwin/fhandler_raw.cc
+++ b/winsup/cygwin/fhandler_raw.cc
@@ -38,7 +38,7 @@ fhandler_dev_raw::fstat (struct stat *buf)
   debug_printf ("here");
 
   fhandler_base::fstat (buf);
-  if (is_auto_device ())
+  if (!dev ().isfs ())
     {
       if (get_major () == DEV_TAPE_MAJOR)
  buf->st_mode = S_IFCHR | STD_RBITS | STD_WBITS | S_IWGRP | S_IWOTH;
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/4] Cygwin: remove path_conv::is_auto_device()

Ken Brown-6
In reply to this post by Ken Brown-6
It is used only once, and the name is supposed to suggest "device that
is not based on the filesystem".  This intended meaning is clearer if
we just replace is_auto_device() by its definition at the place where
it's used.
---
 winsup/cygwin/path.cc | 2 +-
 winsup/cygwin/path.h  | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 158f1e5fb..ed58e966f 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -1921,7 +1921,7 @@ symlink_worker (const char *oldpath, const char *newpath, bool isdevice)
       win32_newpath.get_nt_native_path (), wsym_type);
 
       if ((!isdevice && win32_newpath.exists ())
-  || win32_newpath.is_auto_device ())
+  || (win32_newpath.isdevice () && !win32_newpath.is_fs_special ()))
  {
   set_errno (EEXIST);
   __leave;
diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
index 0c94c6152..d1be1dba0 100644
--- a/winsup/cygwin/path.h
+++ b/winsup/cygwin/path.h
@@ -183,7 +183,6 @@ class path_conv
   int isfifo () const {return dev.is_device (FH_FIFO);}
   int isspecial () const {return dev.not_device (FH_FS);}
   int iscygdrive () const {return dev.is_device (FH_CYGDRIVE);}
-  int is_auto_device () const {return isdevice () && !is_fs_special ();}
   int is_fs_device () const {return isdevice () && is_fs_special ();}
   int is_fs_special () const {return dev.is_fs_special ();}
   int is_lnk_special () const {return is_fs_device () || isfifo () || is_lnk_symlink ();}
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/4] Cygwin: remove path_conv::is_fs_device()

Ken Brown-6
In reply to this post by Ken Brown-6
It is used only once.
---
 winsup/cygwin/path.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
index d1be1dba0..2fd9133c4 100644
--- a/winsup/cygwin/path.h
+++ b/winsup/cygwin/path.h
@@ -183,9 +183,9 @@ class path_conv
   int isfifo () const {return dev.is_device (FH_FIFO);}
   int isspecial () const {return dev.not_device (FH_FS);}
   int iscygdrive () const {return dev.is_device (FH_CYGDRIVE);}
-  int is_fs_device () const {return isdevice () && is_fs_special ();}
   int is_fs_special () const {return dev.is_fs_special ();}
-  int is_lnk_special () const {return is_fs_device () || isfifo () || is_lnk_symlink ();}
+  int is_lnk_special () const {return (isdevice () && is_fs_special ())
+      || isfifo () || is_lnk_symlink ();}
 #ifdef __WITH_AF_UNIX
   int issocket () const {return dev.is_device (FH_LOCAL)
  || dev.is_device (FH_UNIX);}
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/4] Cygwin: socket files are not lnk special files

Ken Brown-6
In reply to this post by Ken Brown-6
Change path_conv::is_lnk_special() so that it returns false on socket
files.

is_lnk_special() is called by rename2() in order to deal with special
files (FIFOs and symlinks, for example) whose Win32 names usually have
a ".lnk" suffix.  Socket files do not fall into this category, and
this change prevents ".lnk" from being appended erroneously when such
files are renamed.

Remove a now redundant !pc.issocket() from fhandler_disk_file::link().
---
 winsup/cygwin/fhandler_disk_file.cc | 4 ++--
 winsup/cygwin/path.h                | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
index 193192762..fe4ee6971 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -1190,10 +1190,10 @@ fhandler_disk_file::link (const char *newpath)
   char new_buf[nlen + 5];
   if (!newpc.error)
     {
-      /* If the original file is a lnk special file (except for sockets),
+      /* If the original file is a lnk special file,
  and if the original file has a .lnk suffix, add one to the hardlink
  as well. */
-      if (pc.is_lnk_special () && !pc.issocket ()
+      if (pc.is_lnk_special ()
   && RtlEqualUnicodePathSuffix (pc.get_nt_native_path (),
  &ro_u_lnk, TRUE))
  {
diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
index 2fd9133c4..65cfa7e7c 100644
--- a/winsup/cygwin/path.h
+++ b/winsup/cygwin/path.h
@@ -184,7 +184,9 @@ class path_conv
   int isspecial () const {return dev.not_device (FH_FS);}
   int iscygdrive () const {return dev.is_device (FH_CYGDRIVE);}
   int is_fs_special () const {return dev.is_fs_special ();}
-  int is_lnk_special () const {return (isdevice () && is_fs_special ())
+
+  int is_lnk_special () const {return (isdevice () && is_fs_special ()
+       && !issocket ())
       || isfifo () || is_lnk_symlink ();}
 #ifdef __WITH_AF_UNIX
   int issocket () const {return dev.is_device (FH_LOCAL)
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/4] Fix rename bug with socket files

Corinna Vinschen-2
In reply to this post by Ken Brown-6
Hi Ken,

On Jul 21 01:53, Ken Brown wrote:
> The last patch of this series fixes the bug reported here:
>
>   https://cygwin.com/ml/cygwin/2019-07/msg00139.html
>
> The first three patches do some cleanup of the is**device() methods.

Looks great, thanks for doing that.  Please push.


Corinna


P.S: Did you see https://cygwin.com/ml/cygwin/2019-07/msg00152.html, by
     any chance?  Looks like a regression from the new FIFO code.

--
Corinna Vinschen
Cygwin Maintainer