[PATCH v2 6/8] syscalls.cc: Expose shallow-pathconv unlink_nt

2021-01-20 Thread Ben Wijen
Not having to query file information improves unlink speed.
---
 winsup/cygwin/syscalls.cc | 78 ++-
 1 file changed, 52 insertions(+), 26 deletions(-)

diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index ab0c4c2d6..b5ab6ac5e 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -1272,6 +1272,28 @@ _unlink_ntpc_ (path_conv& pc, bool shareable)
   return status;
 }
 
+NTSTATUS
+unlink_nt (const char *ourname, ULONG eflags)
+{
+  uint32_t opt = PC_SYM_NOFOLLOW | PC_SKIP_SYM_CHECK | PC_SKIP_FS_CHECK;
+  if (!(eflags & FILE_NON_DIRECTORY_FILE))
+opt &= ~PC_SKIP_FS_CHECK;
+
+  path_conv pc (ourname, opt, NULL);
+  if (pc.error || pc.isspecial ())
+return STATUS_CANNOT_DELETE;
+
+  OBJECT_ATTRIBUTES attr;
+  PUNICODE_STRING ntpath = pc.get_nt_native_path ();
+  InitializeObjectAttributes(, ntpath, 0, NULL, NULL);
+  NTSTATUS status = _unlink_nt (, eflags);
+
+  if (!(eflags & FILE_NON_DIRECTORY_FILE))
+status = _unlink_nt_post_dir_check (status, , pc);
+
+  return status;
+}
+
 NTSTATUS
 unlink_ntpc (path_conv )
 {
@@ -1289,37 +1311,41 @@ unlink (const char *ourname)
 {
   int res = -1;
   dev_t devn;
-  NTSTATUS status;
+  NTSTATUS status = unlink_nt (ourname, FILE_NON_DIRECTORY_FILE);
 
-  path_conv win32_name (ourname, PC_SYM_NOFOLLOW, stat_suffixes);
+  if (!NT_SUCCESS (status))
+  {
+path_conv win32_name (ourname, PC_SYM_NOFOLLOW, stat_suffixes);
 
-  if (win32_name.error)
-{
-  set_errno (win32_name.error);
-  goto done;
-}
+if (win32_name.error)
+  {
+set_errno (win32_name.error);
+goto done;
+  }
 
-  devn = win32_name.get_device ();
-  if (isproc_dev (devn))
-{
-  set_errno (EROFS);
-  goto done;
-}
+devn = win32_name.get_device ();
+if (isproc_dev (devn))
+  {
+set_errno (EROFS);
+goto done;
+  }
 
-  if (!win32_name.exists ())
-{
-  debug_printf ("unlinking a nonexistent file");
-  set_errno (ENOENT);
-  goto done;
-}
-  else if (win32_name.isdir ())
-{
-  debug_printf ("unlinking a directory");
-  set_errno (EISDIR);
-  goto done;
-}
+if (!win32_name.exists ())
+  {
+debug_printf ("unlinking a nonexistent file");
+set_errno (ENOENT);
+goto done;
+  }
+else if (win32_name.isdir ())
+  {
+debug_printf ("unlinking a directory");
+set_errno (EISDIR);
+goto done;
+  }
+
+status = unlink_ntpc (win32_name);
+  }
 
-  status = unlink_ntpc (win32_name);
   if (NT_SUCCESS (status))
 res = 0;
   else
-- 
2.30.0



[PATCH v2 8/8] fhandler_disk_file.cc: Use path_conv's IndexNumber

2021-01-20 Thread Ben Wijen
path_conv already knows the IndexNumber, so just use it.

This commit also fixes the potential handle leak.
---
 winsup/cygwin/fhandler_disk_file.cc | 24 ++--
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/winsup/cygwin/fhandler_disk_file.cc 
b/winsup/cygwin/fhandler_disk_file.cc
index fe04f832b..39f914a59 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -2029,9 +2029,6 @@ readdir_get_ino (const char *path, bool dot_dot)
 {
   char *fname;
   struct stat st;
-  HANDLE hdl;
-  OBJECT_ATTRIBUTES attr;
-  IO_STATUS_BLOCK io;
   ino_t ino = 0;
 
   if (dot_dot)
@@ -2044,26 +2041,17 @@ readdir_get_ino (const char *path, bool dot_dot)
   path = fname;
 }
   path_conv pc (path, PC_SYM_NOFOLLOW | PC_POSIX | PC_KEEP_HANDLE);
-  if (pc.isspecial ())
+  if (pc.isgood_inode (pc.fai ()->InternalInformation.IndexNumber.QuadPart))
+ino = pc.fai ()->InternalInformation.IndexNumber.QuadPart;
+  else if (pc.isspecial ())
 {
   if (!stat_worker (pc, ))
ino = st.st_ino;
 }
-  else if (!pc.hasgood_inode ())
+
+  if (!ino)
 ino = hash_path_name (0, pc.get_nt_native_path ());
-  else if ((hdl = pc.handle ()) != NULL
-  || NT_SUCCESS (NtOpenFile (, READ_CONTROL,
- pc.get_object_attr (attr, sec_none_nih),
- , FILE_SHARE_VALID_FLAGS,
- FILE_OPEN_FOR_BACKUP_INTENT
- | (pc.is_known_reparse_point ()
- ? FILE_OPEN_REPARSE_POINT : 0)))
- )
-{
-  ino = pc.get_ino_by_handle (hdl);
-  if (!ino)
-   ino = hash_path_name (0, pc.get_nt_native_path ());
-}
+
   return ino;
 }
 
-- 
2.30.0



[PATCH v2 7/8] dir.cc: Try unlink_nt first

2021-01-20 Thread Ben Wijen
Speedup deletion of directories.
---
 winsup/cygwin/dir.cc | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/dir.cc b/winsup/cygwin/dir.cc
index 7762557d6..470f83aee 100644
--- a/winsup/cygwin/dir.cc
+++ b/winsup/cygwin/dir.cc
@@ -22,6 +22,8 @@ details. */
 #include "cygtls.h"
 #include "tls_pbuf.h"
 
+extern NTSTATUS unlink_nt (const char *ourname, ULONG eflags);
+
 extern "C" int
 dirfd (DIR *dir)
 {
@@ -398,6 +400,14 @@ rmdir (const char *dir)
  if (msdos && p == dir + 1 && isdrive (dir))
p[1] = '\\';
}
+  if (has_dot_last_component (dir, false)) {
+set_errno (EINVAL);
+__leave;
+  }
+  if (NT_SUCCESS (unlink_nt (dir, FILE_DIRECTORY_FILE))) {
+res = 0;
+__leave;
+  }
   if (!(fh = build_fh_name (dir, PC_SYM_NOFOLLOW)))
__leave;   /* errno already set */;
 
@@ -408,8 +418,6 @@ rmdir (const char *dir)
}
   else if (!fh->exists ())
set_errno (ENOENT);
-  else if (has_dot_last_component (dir, false))
-   set_errno (EINVAL);
   else if (!fh->rmdir ())
res = 0;
   delete fh;
-- 
2.30.0



[PATCH v2 5/8] path.cc: Allow to skip filesystem checks

2021-01-20 Thread Ben Wijen
When file attributes are of no concern, there is no point to query them.
This can greatly speedup code which doesn't need it.

The idea is to have a shallow path conversion with only minimal information.

The upcoming unlink_nt for example, first tries a path without filesystem
checks, then - if necessary - retries with filesystem checks.
---
 winsup/cygwin/path.cc | 7 ---
 winsup/cygwin/path.h  | 2 ++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index abd3687df..441fe113b 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -627,7 +627,7 @@ path_conv::check (const char *src, unsigned opt,
   char *pathbuf = tp.c_get ();
   char *tmp_buf = tp.t_get ();
   char *THIS_path = tp.c_get ();
-  symlink_info sym;
+  symlink_info sym = { 0 };
   bool need_directory = 0;
   bool add_ext = false;
   bool is_relpath;
@@ -931,7 +931,8 @@ path_conv::check (const char *src, unsigned opt,
 
 is_fs_via_procsys:
 
- symlen = sym.check (full_path, suff, fs, conv_handle);
+ if (!(opt & PC_SKIP_SYM_CHECK))
+   symlen = sym.check (full_path, suff, fs, conv_handle);
 
 is_virtual_symlink:
 
@@ -1172,7 +1173,7 @@ path_conv::check (const char *src, unsigned opt,
  return;
}
 
-  if (dev.isfs ())
+  if (!(opt & PC_SKIP_FS_CHECK) && dev.isfs ())
{
  /* If FS hasn't been checked already in symlink_info::check,
 do so now. */
diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
index 62bd5ddd5..5821cdf57 100644
--- a/winsup/cygwin/path.h
+++ b/winsup/cygwin/path.h
@@ -59,6 +59,8 @@ enum pathconv_arg
   PC_KEEP_HANDLE= _BIT (12),   /* keep handle for later stat calls */
   PC_NO_ACCESS_CHECK= _BIT (13),   /* helper flag for error check */
   PC_SYM_NOFOLLOW_DIR   = _BIT (14),   /* don't follow a trailing slash */
+  PC_SKIP_SYM_CHECK = _BIT (15),   /* skip symlink_info::check */
+  PC_SKIP_FS_CHECK  = _BIT (16),   /* skip fs::update check */
   PC_DONT_USE   = _BIT (31)/* conversion to signed happens. */
 };
 
-- 
2.30.0



[PATCH v2 4/8] syscalls.cc: Implement non-path_conv dependent _unlink_nt

2021-01-20 Thread Ben Wijen
Implement _unlink_nt: wich does not depend on patch_conv
---
 winsup/cygwin/fhandler_disk_file.cc |   4 +-
 winsup/cygwin/forkable.cc   |   4 +-
 winsup/cygwin/syscalls.cc   | 211 ++--
 3 files changed, 200 insertions(+), 19 deletions(-)

diff --git a/winsup/cygwin/fhandler_disk_file.cc 
b/winsup/cygwin/fhandler_disk_file.cc
index 07f9c513a..fe04f832b 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -1837,7 +1837,7 @@ fhandler_disk_file::mkdir (mode_t mode)
 int
 fhandler_disk_file::rmdir ()
 {
-  extern NTSTATUS unlink_nt (path_conv );
+  extern NTSTATUS unlink_ntpc (path_conv );
 
   if (!pc.isdir ())
 {
@@ -1850,7 +1850,7 @@ fhandler_disk_file::rmdir ()
   return -1;
 }
 
-  NTSTATUS status = unlink_nt (pc);
+  NTSTATUS status = unlink_ntpc (pc);
 
   if (!NT_SUCCESS (status))
 {
diff --git a/winsup/cygwin/forkable.cc b/winsup/cygwin/forkable.cc
index 350a95c3e..bd7421bf5 100644
--- a/winsup/cygwin/forkable.cc
+++ b/winsup/cygwin/forkable.cc
@@ -29,7 +29,7 @@ details. */
 
 /* Allow concurrent processes to use the same dll or exe
  * via their hardlink while we delete our hardlink. */
-extern NTSTATUS unlink_nt_shareable (path_conv );
+extern NTSTATUS unlink_ntpc_shareable (path_conv );
 
 #define MUTEXSEP L"@"
 #define PATHSEP L"\\"
@@ -132,7 +132,7 @@ rmdirs (WCHAR ntmaxpathbuf[NT_MAX_PATH])
  RtlInitUnicodeString (, ntmaxpathbuf);
 
  path_conv pc ();
- unlink_nt_shareable (pc); /* move to bin */
+ unlink_ntpc_shareable (pc); /* move to bin */
}
 
  if (!pfdi->NextEntryOffset)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index b220bedae..ab0c4c2d6 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -491,7 +491,7 @@ try_to_bin (path_conv , HANDLE , ACCESS_MASK access, 
ULONG flags)
   break;
 case STATUS_DIRECTORY_NOT_EMPTY:
   /* Uh oh!  This was supposed to be avoided by the check_dir_not_empty
-test in unlink_nt, but given that the test isn't atomic, this *can*
+test in unlink_ntpc, but given that the test isn't atomic, this *can*
 happen.  Try to move the dir back ASAP. */
   pfri->RootDirectory = NULL;
   pfri->FileNameLength = pc.get_nt_native_path ()->Length;
@@ -501,7 +501,7 @@ try_to_bin (path_conv , HANDLE , ACCESS_MASK access, 
ULONG flags)
   if (NT_SUCCESS (NtSetInformationFile (fh, , pfri, frisiz,
FileRenameInformation)))
{
- /* Give notice to unlink_nt and leave immediately.  This avoids
+ /* Give notice to unlink_ntpc and leave immediately.  This avoids
 closing the handle, which might still be used if called from
 the rm -r workaround code. */
  bin_stat = dir_not_empty;
@@ -545,7 +545,7 @@ try_to_bin (path_conv , HANDLE , ACCESS_MASK access, 
ULONG flags)
   if ((access & FILE_WRITE_ATTRIBUTES) && NT_SUCCESS (status) && !pc.isdir ())
 NtSetAttributesFile (fh, pc.file_attributes ());
   NtClose (fh);
-  fh = NULL; /* So unlink_nt doesn't close the handle twice. */
+  fh = NULL; /* So unlink_ntpc doesn't close the handle twice. */
   /* On success or when trying to unlink a directory we just return here.
  The below code only works for files.  It also fails on NFS. */
   if (NT_SUCCESS (status) || pc.isdir () || pc.fs_is_nfs ())
@@ -695,7 +695,157 @@ _unlink_nt_post_dir_check (NTSTATUS status, 
POBJECT_ATTRIBUTES attr, const path_
 }
 
 static NTSTATUS
-_unlink_nt (path_conv , bool shareable)
+_unlink_nt (POBJECT_ATTRIBUTES attr, ULONG eflags)
+{
+  static bool has_posix_unlink_semantics =
+  wincap.has_posix_unlink_semantics ();
+  static bool has_posix_unlink_semantics_with_ignore_readonly =
+  wincap.has_posix_unlink_semantics_with_ignore_readonly ();
+
+  HANDLE fh;
+  ACCESS_MASK access = DELETE;
+  IO_STATUS_BLOCK io;
+  ULONG flags = FILE_OPEN_REPARSE_POINT | FILE_OPEN_FOR_BACKUP_INTENT
+  | FILE_DELETE_ON_CLOSE | eflags;
+  NTSTATUS fstatus, istatus = STATUS_SUCCESS;
+
+  syscall_printf("Trying to delete %S, isdir = %d", attr->ObjectName,
+ eflags == FILE_DIRECTORY_FILE);
+
+  //FILE_DELETE_ON_CLOSE icw FILE_DIRECTORY_FILE only works when directory is 
empty
+  //-> We must assume directory not empty, therefore only do this for regular 
files.
+  if (eflags & FILE_NON_DIRECTORY_FILE)
+{
+  //Step 1
+  //If the file is not 'in use' and not 'readonly', this should just work.
+  fstatus = NtOpenFile (, access, attr, , FILE_SHARE_DELETE, flags);
+  debug_printf ("NtOpenFile %S: %y", attr->ObjectName, fstatus);
+}
+
+  if (!(eflags & FILE_NON_DIRECTORY_FILE)// Workaround for the 
non-empty-dir issue
+  || fstatus == STATUS_SHARING_VIOLATION // The file is 'in use'
+  || fstatus == STATUS_CANNOT_DELETE)// The file is 'readonly'
+{
+  //Step 2
+   

[PATCH v2 3/8] Cygwin: Move post-dir unlink check

2021-01-20 Thread Ben Wijen
Move post-dir unlink check from fhandler_disk_file::rmdir to
_unlink_nt_post_dir_check

If a directory is not removed through fhandler_disk_file::rmdir
we can now make sure the post dir check is performed.
---
 winsup/cygwin/fhandler_disk_file.cc | 20 
 winsup/cygwin/syscalls.cc   | 28 
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/winsup/cygwin/fhandler_disk_file.cc 
b/winsup/cygwin/fhandler_disk_file.cc
index 885b59161..07f9c513a 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -1852,26 +1852,6 @@ fhandler_disk_file::rmdir ()
 
   NTSTATUS status = unlink_nt (pc);
 
-  /* Check for existence of remote dirs after trying to delete them.
- Two reasons:
- - Sometimes SMB indicates failure when it really succeeds.
- - Removing a directory on a Samba drive using an old Samba version
-   sometimes doesn't return an error, if the directory can't be removed
-   because it's not empty. */
-  if (isremote ())
-{
-  OBJECT_ATTRIBUTES attr;
-  FILE_BASIC_INFORMATION fbi;
-  NTSTATUS q_status;
-
-  q_status = NtQueryAttributesFile (pc.get_object_attr (attr, 
sec_none_nih),
-   );
-  if (!NT_SUCCESS (status) && q_status == STATUS_OBJECT_NAME_NOT_FOUND)
-   status = STATUS_SUCCESS;
-  else if (pc.fs_is_samba ()
-  && NT_SUCCESS (status) && NT_SUCCESS (q_status))
-   status = STATUS_DIRECTORY_NOT_EMPTY;
-}
   if (!NT_SUCCESS (status))
 {
   __seterrno_from_nt_status (status);
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 54b065733..b220bedae 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -670,6 +670,30 @@ check_dir_not_empty (HANDLE dir, path_conv )
   return STATUS_SUCCESS;
 }
 
+static NTSTATUS
+_unlink_nt_post_dir_check (NTSTATUS status, POBJECT_ATTRIBUTES attr, const 
path_conv )
+{
+  /* Check for existence of remote dirs after trying to delete them.
+ Two reasons:
+ - Sometimes SMB indicates failure when it really succeeds.
+ - Removing a directory on a Samba drive using an old Samba version
+   sometimes doesn't return an error, if the directory can't be removed
+   because it's not empty. */
+  if (pc.isremote ())
+{
+  FILE_BASIC_INFORMATION fbi;
+  NTSTATUS q_status;
+
+  q_status = NtQueryAttributesFile (attr, );
+  if (!NT_SUCCESS (status) && q_status == STATUS_OBJECT_NAME_NOT_FOUND)
+  status = STATUS_SUCCESS;
+  else if (pc.fs_is_samba ()
+   && NT_SUCCESS (status) && NT_SUCCESS (q_status))
+  status = STATUS_DIRECTORY_NOT_EMPTY;
+}
+  return status;
+}
+
 static NTSTATUS
 _unlink_nt (path_conv , bool shareable)
 {
@@ -1059,6 +1083,10 @@ out:
   /* Stop transaction if we started one. */
   if (trans)
 stop_transaction (status, old_trans, trans);
+
+  if (pc.isdir ())
+status = _unlink_nt_post_dir_check (status, , pc);
+
   syscall_printf ("%S, return status = %y", pc.get_nt_native_path (), status);
   return status;
 }
-- 
2.30.0



[PATCH v2 2/8] syscalls.cc: Deduplicate remove

2021-01-20 Thread Ben Wijen
The remove code is already in the _remove_r function.
So, just call the _remove_r function.
---
 winsup/cygwin/syscalls.cc | 17 -
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 2e50ad7d5..54b065733 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -1133,24 +1133,15 @@ _remove_r (struct _reent *, const char *ourname)
   return -1;
 }
 
-  return win32_name.isdir () ? rmdir (ourname) : unlink (ourname);
+  int res = win32_name.isdir () ? rmdir (ourname) : unlink (ourname);
+  syscall_printf ("%R = remove(%s)", res, ourname);
+  return res;
 }
 
 extern "C" int
 remove (const char *ourname)
 {
-  path_conv win32_name (ourname, PC_SYM_NOFOLLOW);
-
-  if (win32_name.error)
-{
-  set_errno (win32_name.error);
-  syscall_printf ("-1 = remove (%s)", ourname);
-  return -1;
-}
-
-  int res = win32_name.isdir () ? rmdir (ourname) : unlink (ourname);
-  syscall_printf ("%R = remove(%s)", res, ourname);
-  return res;
+  return _remove_r(_REENT, ourname);
 }
 
 extern "C" pid_t
-- 
2.30.0



[PATCH v2 1/8] syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE

2021-01-20 Thread Ben Wijen
Implement wincap.has_posix_unlink_semantics_with_ignore_readonly and when set
skip setting/clearing of READONLY attribute and instead use
FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE
---
 winsup/cygwin/ntdll.h |  3 ++-
 winsup/cygwin/syscalls.cc | 14 +-
 winsup/cygwin/wincap.cc   | 11 
 winsup/cygwin/wincap.h| 56 ---
 4 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/winsup/cygwin/ntdll.h b/winsup/cygwin/ntdll.h
index d4f6aaf45..7eee383dd 100644
--- a/winsup/cygwin/ntdll.h
+++ b/winsup/cygwin/ntdll.h
@@ -497,7 +497,8 @@ enum {
   FILE_DISPOSITION_DELETE  = 0x01,
   FILE_DISPOSITION_POSIX_SEMANTICS = 0x02,
   FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK   = 0x04,
-  FILE_DISPOSITION_ON_CLOSE= 0x08
+  FILE_DISPOSITION_ON_CLOSE= 0x08,
+  FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE   = 0x10,
 };
 
 enum
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 4742c6653..2e50ad7d5 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -709,14 +709,11 @@ _unlink_nt (path_conv , bool shareable)
   flags);
   if (!NT_SUCCESS (status))
goto out;
-  /* Why didn't the devs add a FILE_DELETE_IGNORE_READONLY_ATTRIBUTE
-flag just like they did with FILE_LINK_IGNORE_READONLY_ATTRIBUTE
-and FILE_LINK_IGNORE_READONLY_ATTRIBUTE???
-
- POSIX unlink semantics are nice, but they still fail if the file
+  /* POSIX unlink semantics are nice, but they still fail if the file
 has the R/O attribute set.  Removing the file is very much a safe
 bet afterwards, so, no transaction. */
-  if (pc.file_attributes () & FILE_ATTRIBUTE_READONLY)
+  if (!wincap.has_posix_unlink_semantics_with_ignore_readonly ()
+  && (pc.file_attributes () & FILE_ATTRIBUTE_READONLY))
{
  status = NtSetAttributesFile (fh, pc.file_attributes ()
& ~FILE_ATTRIBUTE_READONLY);
@@ -727,10 +724,13 @@ _unlink_nt (path_conv , bool shareable)
}
}
   fdie.Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS;
+  if(wincap.has_posix_unlink_semantics_with_ignore_readonly ())
+  fdie.Flags |= FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE;
   status = NtSetInformationFile (fh, , , sizeof fdie,
 FileDispositionInformationEx);
   /* Restore R/O attribute in case we have multiple hardlinks. */
-  if (pc.file_attributes () & FILE_ATTRIBUTE_READONLY)
+  if (!wincap.has_posix_unlink_semantics_with_ignore_readonly ()
+  && (pc.file_attributes () & FILE_ATTRIBUTE_READONLY))
NtSetAttributesFile (fh, pc.file_attributes ());
   NtClose (fh);
   /* Trying to delete in-use executables and DLLs using
diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc
index b18e732cd..635e0892b 100644
--- a/winsup/cygwin/wincap.cc
+++ b/winsup/cygwin/wincap.cc
@@ -38,6 +38,7 @@ wincaps wincap_vista __attribute__((section 
(".cygwin_dll_common"), shared)) = {
 has_unbiased_interrupt_time:false,
 has_precise_interrupt_time:false,
 has_posix_unlink_semantics:false,
+has_posix_unlink_semantics_with_ignore_readonly:false,
 has_case_sensitive_dirs:false,
 has_posix_rename_semantics:false,
 no_msv1_0_s4u_logon_in_wow64:true,
@@ -72,6 +73,7 @@ wincaps wincap_7 __attribute__((section 
(".cygwin_dll_common"), shared)) = {
 has_unbiased_interrupt_time:true,
 has_precise_interrupt_time:false,
 has_posix_unlink_semantics:false,
+has_posix_unlink_semantics_with_ignore_readonly:false,
 has_case_sensitive_dirs:false,
 has_posix_rename_semantics:false,
 no_msv1_0_s4u_logon_in_wow64:true,
@@ -106,6 +108,7 @@ wincaps wincap_8 __attribute__((section 
(".cygwin_dll_common"), shared)) = {
 has_unbiased_interrupt_time:true,
 has_precise_interrupt_time:false,
 has_posix_unlink_semantics:false,
+has_posix_unlink_semantics_with_ignore_readonly:false,
 has_case_sensitive_dirs:false,
 has_posix_rename_semantics:false,
 no_msv1_0_s4u_logon_in_wow64:false,
@@ -140,6 +143,7 @@ wincaps wincap_8_1 __attribute__((section 
(".cygwin_dll_common"), shared)) = {
 has_unbiased_interrupt_time:true,
 has_precise_interrupt_time:false,
 has_posix_unlink_semantics:false,
+has_posix_unlink_semantics_with_ignore_readonly:false,
 has_case_sensitive_dirs:false,
 has_posix_rename_semantics:false,
 no_msv1_0_s4u_logon_in_wow64:false,
@@ -174,6 +178,7 @@ wincaps  wincap_10_1507 __attribute__((section 
(".cygwin_dll_common"), shared))
 has_unbiased_interrupt_time:true,
 has_precise_interrupt_time:true,
 has_posix_unlink_semantics:false,
+has_posix_unlink_semantics_with_ignore_readonly:false,
 

[PATCH v2 0/8] Improve rm speed

2021-01-20 Thread Ben Wijen
Hi,

I think I got all remarks, please let me know if I missed something.

I'm still thinking on a better way to use fs_info::update cache,
but it requires more testing.


Thank you,

Ben Wijen (8):
  syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE
  syscalls.cc: Deduplicate remove
  Cygwin: Move post-dir unlink check
  syscalls.cc: Implement non-path_conv dependent _unlink_nt
  path.cc: Allow to skip filesystem checks
  syscalls.cc: Expose shallow-pathconv unlink_nt
  dir.cc: Try unlink_nt first
  fhandler_disk_file.cc: Use path_conv's IndexNumber

 winsup/cygwin/dir.cc|  12 +-
 winsup/cygwin/fhandler_disk_file.cc |  48 +---
 winsup/cygwin/forkable.cc   |   4 +-
 winsup/cygwin/ntdll.h   |   3 +-
 winsup/cygwin/path.cc   |   7 +-
 winsup/cygwin/path.h|   2 +
 winsup/cygwin/syscalls.cc   | 346 +++-
 winsup/cygwin/wincap.cc |  11 +
 winsup/cygwin/wincap.h  |  56 ++---
 9 files changed, 354 insertions(+), 135 deletions(-)

-- 
2.30.0



[PATCH] Cygwin: normalize_posix_path: fix error handling when .. is encountered

2021-01-20 Thread Ken Brown via Cygwin-patches
When .. is in the source path and the path prefix exists but is not a
directory, return ENOTDIR instead of ENOENT.  This fixes a failing
gnulib test of realpath(3).

Addresses: https://lists.gnu.org/archive/html/bug-gnulib/2021-01/msg00214.html
---
 winsup/cygwin/path.cc   | 4 +++-
 winsup/cygwin/release/3.2.0 | 4 
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index abd3687df..6dc162806 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -323,8 +323,10 @@ normalize_posix_path (const char *src, char *dst, char 
*)
  if (!tp.check_usage (4, 3))
return ELOOP;
  path_conv head (dst, PC_SYM_FOLLOW | PC_POSIX);
- if (!head.isdir())
+ if (!head.exists ())
return ENOENT;
+ if (!head.isdir ())
+   return ENOTDIR;
  /* At this point, dst is a normalized path.  If the
 normalized path created by path_conv does not
 match the normalized path we're just testing, then
diff --git a/winsup/cygwin/release/3.2.0 b/winsup/cygwin/release/3.2.0
index c18a848de..43725cec2 100644
--- a/winsup/cygwin/release/3.2.0
+++ b/winsup/cygwin/release/3.2.0
@@ -48,3 +48,7 @@ Bug Fixes
 
 - Fix a bug in fstatat(2) on 32 bit that could cause it to return garbage.
   Addresses: https://cygwin.com/pipermail/cygwin/2021-January/247399.html
+
+- Fix the errno when a path contains .. and the prefix exists but is
+  not a directory.
+  Addresses: 
https://lists.gnu.org/archive/html/bug-gnulib/2021-01/msg00214.html
-- 
2.30.0



Re: [PATCH] Cygwin: pty: Reduce buffer size in get_console_process_id().

2021-01-20 Thread Corinna Vinschen via Cygwin-patches
On Jan 20 19:40, Takashi Yano via Cygwin-patches wrote:
> On Wed, 20 Jan 2021 10:50:24 +0100
> Corinna Vinschen wrote:
> > On Jan 20 09:57, Takashi Yano via Cygwin-patches wrote:
> > > - The buffer used in get_console_process_id(), introduced by commit
> > >   72770148, is too large and ERROR_NOT_ENOUGH_MEMORY occurs in Win7.
> > 
> > Huh, funny!  Will we ever be happy with just 8192 processes per
> > console? :)
> 
> According to my test, when the buffer size is larger than 15683,
> this error occurs. Test environment is Win7 x64. Both inside and
> outside of WOW64, the maximum allowed size seems to be the same.
> 
> Shall we increase to 15683? :p

Ugh, please, no :D

8K processes per console is fine.  The machine will probably be out of
memory before this is a concern.


Corinna


Re: [PATCH] Cygwin: pty: Reduce buffer size in get_console_process_id().

2021-01-20 Thread Takashi Yano via Cygwin-patches
On Wed, 20 Jan 2021 10:50:24 +0100
Corinna Vinschen wrote:
> On Jan 20 09:57, Takashi Yano via Cygwin-patches wrote:
> > - The buffer used in get_console_process_id(), introduced by commit
> >   72770148, is too large and ERROR_NOT_ENOUGH_MEMORY occurs in Win7.
> 
> Huh, funny!  Will we ever be happy with just 8192 processes per
> console? :)

According to my test, when the buffer size is larger than 15683,
this error occurs. Test environment is Win7 x64. Both inside and
outside of WOW64, the maximum allowed size seems to be the same.

Shall we increase to 15683? :p

-- 
Takashi Yano 


Re: [PATCH] Cygwin: console: Fix "Bad file descriptor" error in script command.

2021-01-20 Thread Corinna Vinschen via Cygwin-patches
On Jan 20 18:16, Takashi Yano via Cygwin-patches wrote:
> - After the commit 72770148, script command exits occasionally with
>   the error "Bad file descriptor" if it is started in console on Win7
>   and non-cygwin process is executed. This patch fixes the issue.
> ---
>  winsup/cygwin/fhandler_console.cc | 10 ++--
>  winsup/cygwin/select.cc   | 95 ++-
>  winsup/cygwin/select.h|  7 +++
>  3 files changed, 105 insertions(+), 7 deletions(-)

Pushed.


Thanks,
Corinna


Re: [PATCH] Cygwin: pty: Reduce buffer size in get_console_process_id().

2021-01-20 Thread Corinna Vinschen via Cygwin-patches
On Jan 20 09:57, Takashi Yano via Cygwin-patches wrote:
> - The buffer used in get_console_process_id(), introduced by commit
>   72770148, is too large and ERROR_NOT_ENOUGH_MEMORY occurs in Win7.

Huh, funny!  Will we ever be happy with just 8192 processes per
console? :)

>   Therefore, the buffer size has been reduced.
> ---
>  winsup/cygwin/fhandler_tty.cc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Pushed.


THanks,
Corinna


[PATCH] Cygwin: console: Fix "Bad file descriptor" error in script command.

2021-01-20 Thread Takashi Yano via Cygwin-patches
- After the commit 72770148, script command exits occasionally with
  the error "Bad file descriptor" if it is started in console on Win7
  and non-cygwin process is executed. This patch fixes the issue.
---
 winsup/cygwin/fhandler_console.cc | 10 ++--
 winsup/cygwin/select.cc   | 95 ++-
 winsup/cygwin/select.h|  7 +++
 3 files changed, 105 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc
index dd00079fa..49963e719 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -557,9 +557,11 @@ fhandler_console::read (void *pv, size_t& buflen)
 #define buf ((char *) pv)
 
   int ret;
+  acquire_attach_mutex (INFINITE);
   acquire_input_mutex (INFINITE);
   ret = process_input_message ();
   release_input_mutex ();
+  release_attach_mutex ();
   switch (ret)
{
case input_error:
@@ -616,8 +618,6 @@ fhandler_console::process_input_message (void)
   if (!shared_console_info)
 return input_error;
 
-  acquire_attach_mutex (INFINITE);
-
   termios *ti = &(get_ttyp ()->ti);
 
   fhandler_console::input_states stat = input_processing;
@@ -627,7 +627,6 @@ fhandler_console::process_input_message (void)
   if (!PeekConsoleInputW (get_handle (), input_rec, INREC_SIZE, _read))
 {
   termios_printf ("PeekConsoleInput failed, %E");
-  release_attach_mutex ();
   return input_error;
 }
 
@@ -991,8 +990,9 @@ fhandler_console::process_input_message (void)
 out:
   /* Discard processed recored. */
   DWORD dummy;
-  ReadConsoleInputW (get_handle (), input_rec, min (total_read, i+1), );
-  release_attach_mutex ();
+  DWORD discard_len = min (total_read, i + 1);
+  if (discard_len)
+ReadConsoleInputW (get_handle (), input_rec, discard_len, );
   return stat;
 }
 
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index dcb9b2d6e..d6c13241e 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -1032,6 +1032,22 @@ fhandler_fifo::select_except (select_stuff *ss)
   return s;
 }
 
+extern HANDLE attach_mutex; /* Defined in fhandler_console.cc */
+
+static inline void
+acquire_attach_mutex (DWORD t)
+{
+  if (attach_mutex)
+WaitForSingleObject (attach_mutex, t);
+}
+
+static inline void
+release_attach_mutex ()
+{
+  if (attach_mutex)
+ReleaseMutex (attach_mutex);
+}
+
 static int
 peek_console (select_record *me, bool)
 {
@@ -1057,10 +1073,14 @@ peek_console (select_record *me, bool)
   HANDLE h;
   set_handle_or_return_if_not_open (h, me);
 
+  acquire_attach_mutex (INFINITE);
   while (!fh->input_ready && !fh->get_cons_readahead_valid ())
 {
   if (fh->bg_check (SIGTTIN, true) <= bg_eof)
-   return me->read_ready = true;
+   {
+ release_attach_mutex ();
+ return me->read_ready = true;
+   }
   else if (!PeekConsoleInputW (h, , 1, _read) || !events_read)
break;
   fh->acquire_input_mutex (INFINITE);
@@ -1070,10 +1090,12 @@ peek_console (select_record *me, bool)
{
  set_sig_errno (EINTR);
  fh->release_input_mutex ();
+ release_attach_mutex ();
  return -1;
}
   fh->release_input_mutex ();
 }
+  release_attach_mutex ();
   if (fh->input_ready || fh->get_cons_readahead_valid ())
 return me->read_ready = true;
 
@@ -1087,18 +1109,87 @@ verify_console (select_record *me, fd_set *rfds, fd_set 
*wfds,
   return peek_console (me, true);
 }
 
+static int console_startup (select_record *me, select_stuff *stuff);
+
+static DWORD WINAPI
+thread_console (void *arg)
+{
+  select_console_info *ci = (select_console_info *) arg;
+  DWORD sleep_time = 0;
+  bool looping = true;
+
+  while (looping)
+{
+  for (select_record *s = ci->start; (s = s->next); )
+   if (s->startup == console_startup)
+ {
+   if (peek_console (s, true))
+ looping = false;
+   if (ci->stop_thread)
+ {
+   select_printf ("stopping");
+   looping = false;
+   break;
+ }
+ }
+  if (!looping)
+   break;
+  cygwait (ci->bye, sleep_time >> 3);
+  if (sleep_time < 80)
+   ++sleep_time;
+  if (ci->stop_thread)
+   break;
+}
+  return 0;
+}
+
 static int
 console_startup (select_record *me, select_stuff *stuff)
 {
   fhandler_console *fh = (fhandler_console *) me->fh;
   if (wincap.has_con_24bit_colors ())
 fhandler_console::request_xterm_mode_input (true, fh->get_handle_set ());
+
+  select_console_info *ci = stuff->device_specific_console;
+  if (ci->start)
+me->h = *(stuff->device_specific_console)->thread;
+  else
+{
+  ci->start = >start;
+  ci->stop_thread = false;
+  ci->bye = CreateEvent (_none_nih, TRUE, FALSE, NULL);
+  ci->thread = new cygthread (thread_console, ci, "conssel");
+  me->h = *ci->thread;
+  if (!me->h)
+   return 0;