Re: [PATCH] mkdir: always check-for-existence

Hi Corinna, Please see the attachment for my patch. My MUA indeed replaced the tabs with spaces. I did notice that the indentation was mixed tabs and spaces, but as stated on the website I have kept the surrounding indentation. Ben... On 04-06-2019 09:41, Corinna Vinschen wrote: Hi Ben

[PATCH] mkdir: alway check-for-existence

:\’ for which both of these functions return ‘ERROR_ACCESS_DENIED’ We could call NtCreateFile with 'FILE_OPEN_IF' instead of 'FILE_CREATE' but since that's an internal function we better always check for existence ourselves. Signed-off-by: Ben Wijen ---  winsup/cygwin/dir.cc | 4 +++-  1 file changed

Re: [PATCH] mkdir: always check-for-existence

When creating a directory which already exists, NtCreateFile will correctly return 'STATUS_OBJECT_NAME_COLLISION'. However when creating a directory and all its parents a normal use would be to start with mkdir(‘/cygdrive/c’) which translates to ‘C:\’ for which it'll instead return

Re: [PATCH 05/11] Cygwin: Move post-dir unlink check

On 18-01-2021 12:08, Corinna Vinschen via Cygwin-patches wrote: > On Jan 15 14:45, Ben Wijen wrote: >> Move post-dir unlink check from >> fhandler_disk_file::rmdir to _unlink_nt > > Why? It's not much of a problem, codewise, but the commit message > could be improved

Re: [PATCH 02/11] syscalls.cc: Deduplicate _remove_r

syscall_printf ("%R = remove(%s)", res, ourname); - return res; + return _remove_r (__getreent (), ourname); } extern "C" pid_t Ben...

Re: [PATCH 01/11] syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE first

ix_rename_semantics so I didn't bother adding it. Ben...

Re: [PATCH 02/11] syscalls.cc: Deduplicate _remove_r

ed > here... > > What's your stance? > While I could do without: In an earlier version I had changed remove and missed remove_r. So, this commit is more about de-duplication rather than speed. Ben...

Re: [PATCH 07/11] syscalls.cc: Implement non-path_conv dependent _unlink_nt

Hi, After reiterating over my patches, I see this must come after implementing the 'poor mans cache' commit. I will reorder in a new patch-set. Sorry about that. Ben... On 15-01-2021 14:45, Ben Wijen wrote: > Implement _unlink_nt: wich does not depend on patch_conv > --- > wins

Re: [PATCH 01/11] syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE first

On 18-01-2021 13:22, Corinna Vinschen via Cygwin-patches wrote: > On Jan 18 13:11, Ben wrote: >> >> >> On 18-01-2021 11:45, Corinna Vinschen via Cygwin-patches wrote: >>> Rather than calling NtSetInformationFile here again, we should rather >>> just skip

Re: [PATCH 02/11] syscalls.cc: Deduplicate _remove_r

On 18-01-2021 15:49, Corinna Vinschen via Cygwin-patches wrote: > > Care to send the resulting patch? > Will send with next patch-set. Ben...

Re: [PATCH 01/11] syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE first

> > I'm sure, but that code path is called on non-remote ntfs only anyway. > Ofcourse, I was thinking about the new _unlink_nt...

Re: [PATCH 11/11] dir.cc: Try unlink_nt first

On 18-01-2021 13:13, Corinna Vinschen via Cygwin-patches wrote: > > Your code is skipping the safety checks and the has_dot_last_component() > check. The latter implements a check required by POSIX. Skipping > it introduces an incompatibility, see man 2 rmdir. > Yes, I missed

Re: [PATCH 08/11] path.cc: Allow to skip filesystem checks

On 18-01-2021 12:36, Corinna Vinschen via Cygwin-patches wrote: > On Jan 15 14:45, Ben Wijen wrote: > > Without any code setting the flag, this doesn't seem to make any > sense. At least the commit message should reflect on the reasons > for this change. > Something like

Re: [PATCH 11/11] dir.cc: Try unlink_nt first

On 18-01-2021 18:07, Ben wrote: > > Have I missed something else? > Alright, I now see unlink uses isproc_dev and rmdir uses isdev_dev. Is this correct? Why are files and directories handled differently? Anyway, I will add isdev_dev to the new unlink_nt Ben...

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

ink version without path_conv and thus, > without certain check, like the reparse point check, or checks for > filesystems with quirks, like MVFS? What about remote filesystems of > which you don't know nothing, e. g., a Win7 NTFS? Did you check the > error codes in this case? > The idea is to - eventually - replace/incorporate the fallback unlink_nt. But, because I indeed don't know the quirks of all filesystems, I left the fallback scenario intact, for now. No, I have not checked all error codes, I simply don't have all filesystems at my disposal, again the reason for keeping the fallback. The general idea is to forgo path_conv's filesystem checks and just try to delete the file, if it fails, remember and fallback. After these series of commits, some will follow to try and see if we can remove/incorporate the fallback scenario completely. Ben...

Re: [PATCH 09/11] mount.cc: Implement poor-man's cache

On 18-01-2021 12:51, Corinna Vinschen via Cygwin-patches wrote: > Ok, so hash_prefix reduces the path to a drive letter or the UNC path > prefix and hashes it. However, what about partitions mounted to a > subdir of, say, drive C? In that case the hashing goes awry, because > you're comparing

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

On 26-01-2021 12:46, Corinna Vinschen via Cygwin-patches wrote: > > So what about /dev, /proc, etc? > The idea was to catch them with pc.isspecial() in unlink_nt.

[PATCH v2 0/8] Improve rm speed

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

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

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

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

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

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

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

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

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

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

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

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

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 @@

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

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

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

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

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

I think we don't need an extra flag as we can utilize: access & FILE_WRITE_ATTRIBUTES What do you think? Ben Wijen (1): syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE winsup/cygwin/ntdll.h | 3 ++- winsup/cygwin/syscalls.cc | 22 +++ winsup/cy

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

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 8651cfade..e6ff0fd7a 100644

[PATCH 01/11] syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE first

--- winsup/cygwin/ntdll.h | 3 ++- winsup/cygwin/syscalls.cc | 20 2 files changed, 18 insertions(+), 5 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 @@

[PATCH 04/11] syscalls.cc: Use EISDIR

This is the non-POSIX value returned by Linux since 2.1.132. --- winsup/cygwin/syscalls.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index 227d1a911..043ccdb99 100644 --- a/winsup/cygwin/syscalls.cc +++

[PATCH 03/11] syscalls.cc: Fix num_links

NtQueryInformationFile on fh_ro needs FILE_READ_ATTRIBUTES to succeed. --- winsup/cygwin/syscalls.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index 0e89b4f44..227d1a911 100644 --- a/winsup/cygwin/syscalls.cc +++

[PATCH 06/11] cxx.cc: Fix dynamic initialization for static local variables

The old implementation for __cxa_guard_acquire did not return 1, therefore dynamic initialization was never performed. If concurrent-safe dynamic initialisation is ever needed, CXX ABI must be followed when re-implementing __cxa_guard_acquire (et al.) --- winsup/cygwin/Makefile.in | 2 +-

[PATCH 07/11] syscalls.cc: Implement non-path_conv dependent _unlink_nt

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 | 239 ++-- 3 files changed, 228 insertions(+), 19 deletions(-) diff --git

[PATCH 08/11] path.cc: Allow to skip filesystem checks

When file attributes are of no concern, there is no point to query them. --- winsup/cygwin/path.cc | 3 +++ winsup/cygwin/path.h | 1 + 2 files changed, 4 insertions(+) diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index abd3687df..f00707e86 100644 --- a/winsup/cygwin/path.cc +++

[PATCH 09/11] mount.cc: Implement poor-man's cache

Try to avoid NtQueryVolumeInformationFile. --- winsup/cygwin/mount.cc | 78 -- winsup/cygwin/mount.h | 2 +- winsup/cygwin/path.cc | 2 +- winsup/cygwin/path.h | 1 + 4 files changed, 56 insertions(+), 27 deletions(-) diff --git

[PATCH 10/11] syscalls.cc: Expose shallow-pathconv unlink_nt

Not having to query file information improves unlink speed. --- winsup/cygwin/syscalls.cc | 68 --- 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index 79e4a4422..8aecdf453 100644 ---

[PATCH 11/11] dir.cc: Try unlink_nt first

Speedup deletion of directories. --- winsup/cygwin/dir.cc | 6 ++ 1 file changed, 6 insertions(+) diff --git a/winsup/cygwin/dir.cc b/winsup/cygwin/dir.cc index f912a9e47..2e7da3638 100644 --- a/winsup/cygwin/dir.cc +++ b/winsup/cygwin/dir.cc @@ -22,6 +22,8 @@ details. */ #include