From: Ben Wijen <ben@wijen.net>
To: cygwin-patches@cygwin.com
Subject: [PATCH v3 1/8] syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE
Date: Fri, 22 Jan 2021 16:47:11 +0100 [thread overview]
Message-ID: <20210122154712.3207-1-ben@wijen.net> (raw)
In-Reply-To: <20210122105201.GD810271@calimero.vinschen.de>
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/cygwin/wincap.cc | 11 ++++++++
winsup/cygwin/wincap.h | 56 ++++++++++++++++++++-------------------
4 files changed, 53 insertions(+), 39 deletions(-)
--
2.30.0
From 2d0ff6fec10d03c24d11c747852018b7bc1136ac Mon Sep 17 00:00:00 2001
In-Reply-To: <20210122105201.GD810271@calimero.vinschen.de>
References: <20210122105201.GD810271@calimero.vinschen.de>
From: Ben Wijen <ben@wijen.net>
Date: Tue, 17 Dec 2019 15:15:25 +0100
Subject: [PATCH v3 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 | 22 +++++++--------
winsup/cygwin/wincap.cc | 11 ++++++++
winsup/cygwin/wincap.h | 56 ++++++++++++++++++++-------------------
4 files changed, 53 insertions(+), 39 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 7044ea903..8651cfade 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -719,7 +719,7 @@ _unlink_nt (path_conv &pc, bool shareable)
pc.get_object_attr (attr, sec_none_nih);
- /* First check if we can use POSIX unlink semantics: W10 1709++, local NTFS.
+ /* First check if we can use POSIX unlink semantics: W10 1709+, local NTFS.
With POSIX unlink semantics the entire job gets MUCH easier and faster.
Just try to do it and if it fails, it fails. */
if (wincap.has_posix_unlink_semantics ()
@@ -727,20 +727,18 @@ _unlink_nt (path_conv &pc, bool shareable)
{
FILE_DISPOSITION_INFORMATION_EX fdie;
- if (pc.file_attributes () & FILE_ATTRIBUTE_READONLY)
+ /* POSIX unlink semantics are nice, but they still fail if the file has
+ the R/O attribute set. If so, ignoring might be an option: W10 1809+
+ Removing the file is very much a safe bet afterwards, so, no
+ transaction. */
+ if ((pc.file_attributes () & FILE_ATTRIBUTE_READONLY)
+ && !wincap.has_posix_unlink_semantics_with_ignore_readonly ())
access |= FILE_WRITE_ATTRIBUTES;
status = NtOpenFile (&fh, access, &attr, &io, FILE_SHARE_VALID_FLAGS,
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
- 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 (access & FILE_WRITE_ATTRIBUTES)
{
status = NtSetAttributesFile (fh, pc.file_attributes ()
& ~FILE_ATTRIBUTE_READONLY);
@@ -751,10 +749,12 @@ _unlink_nt (path_conv &pc, 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, &io, &fdie, sizeof fdie,
FileDispositionInformationEx);
/* Restore R/O attribute in case we have multiple hardlinks. */
- if (pc.file_attributes () & FILE_ATTRIBUTE_READONLY)
+ if (access & FILE_WRITE_ATTRIBUTES)
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,
has_case_sensitive_dirs:false,
has_posix_rename_semantics:false,
no_msv1_0_s4u_logon_in_wow64:false,
@@ -208,6 +213,7 @@ wincaps wincap_10_1607 __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,
has_case_sensitive_dirs:false,
has_posix_rename_semantics:false,
no_msv1_0_s4u_logon_in_wow64:false,
@@ -242,6 +248,7 @@ wincaps wincap_10_1703 __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,
has_case_sensitive_dirs:false,
has_posix_rename_semantics:false,
no_msv1_0_s4u_logon_in_wow64:false,
@@ -276,6 +283,7 @@ wincaps wincap_10_1709 __attribute__((section (".cygwin_dll_common"), shared)) =
has_unbiased_interrupt_time:true,
has_precise_interrupt_time:true,
has_posix_unlink_semantics:true,
+ 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,
@@ -310,6 +318,7 @@ wincaps wincap_10_1803 __attribute__((section (".cygwin_dll_common"), shared)) =
has_unbiased_interrupt_time:true,
has_precise_interrupt_time:true,
has_posix_unlink_semantics:true,
+ has_posix_unlink_semantics_with_ignore_readonly:false,
has_case_sensitive_dirs:true,
has_posix_rename_semantics:false,
no_msv1_0_s4u_logon_in_wow64:false,
@@ -344,6 +353,7 @@ wincaps wincap_10_1809 __attribute__((section (".cygwin_dll_common"), shared)) =
has_unbiased_interrupt_time:true,
has_precise_interrupt_time:true,
has_posix_unlink_semantics:true,
+ has_posix_unlink_semantics_with_ignore_readonly:true,
has_case_sensitive_dirs:true,
has_posix_rename_semantics:true,
no_msv1_0_s4u_logon_in_wow64:false,
@@ -378,6 +388,7 @@ wincaps wincap_10_1903 __attribute__((section (".cygwin_dll_common"), shared)) =
has_unbiased_interrupt_time:true,
has_precise_interrupt_time:true,
has_posix_unlink_semantics:true,
+ has_posix_unlink_semantics_with_ignore_readonly:true,
has_case_sensitive_dirs:true,
has_posix_rename_semantics:true,
no_msv1_0_s4u_logon_in_wow64:false,
diff --git a/winsup/cygwin/wincap.h b/winsup/cygwin/wincap.h
index 2f4191aa1..687e51843 100644
--- a/winsup/cygwin/wincap.h
+++ b/winsup/cygwin/wincap.h
@@ -16,33 +16,34 @@ struct wincaps
/* The bitfields must be 8 byte aligned on x86_64, otherwise the bitfield
ops generated by gcc are off by 4 bytes. */
struct __attribute__ ((aligned (8))) {
- unsigned is_server : 1;
- unsigned needs_count_in_si_lpres2 : 1;
- unsigned needs_query_information : 1;
- unsigned has_gaa_largeaddress_bug : 1;
- unsigned has_broken_alloc_console : 1;
- unsigned has_console_logon_sid : 1;
- unsigned has_precise_system_time : 1;
- unsigned has_microsoft_accounts : 1;
- unsigned has_processor_groups : 1;
- unsigned has_broken_prefetchvm : 1;
- unsigned has_new_pebteb_region : 1;
- unsigned has_broken_whoami : 1;
- unsigned has_unprivileged_createsymlink : 1;
- unsigned has_unbiased_interrupt_time : 1;
- unsigned has_precise_interrupt_time : 1;
- unsigned has_posix_unlink_semantics : 1;
- unsigned has_case_sensitive_dirs : 1;
- unsigned has_posix_rename_semantics : 1;
- unsigned no_msv1_0_s4u_logon_in_wow64 : 1;
- unsigned has_con_24bit_colors : 1;
- unsigned has_con_broken_csi3j : 1;
- unsigned has_con_broken_il_dl : 1;
- unsigned has_con_esc_rep : 1;
- unsigned has_extended_mem_api : 1;
- unsigned has_tcp_fastopen : 1;
- unsigned has_linux_tcp_keepalive_sockopts : 1;
- unsigned has_tcp_maxrtms : 1;
+ unsigned is_server : 1;
+ unsigned needs_count_in_si_lpres2 : 1;
+ unsigned needs_query_information : 1;
+ unsigned has_gaa_largeaddress_bug : 1;
+ unsigned has_broken_alloc_console : 1;
+ unsigned has_console_logon_sid : 1;
+ unsigned has_precise_system_time : 1;
+ unsigned has_microsoft_accounts : 1;
+ unsigned has_processor_groups : 1;
+ unsigned has_broken_prefetchvm : 1;
+ unsigned has_new_pebteb_region : 1;
+ unsigned has_broken_whoami : 1;
+ unsigned has_unprivileged_createsymlink : 1;
+ unsigned has_unbiased_interrupt_time : 1;
+ unsigned has_precise_interrupt_time : 1;
+ unsigned has_posix_unlink_semantics : 1;
+ unsigned has_posix_unlink_semantics_with_ignore_readonly : 1;
+ unsigned has_case_sensitive_dirs : 1;
+ unsigned has_posix_rename_semantics : 1;
+ unsigned no_msv1_0_s4u_logon_in_wow64 : 1;
+ unsigned has_con_24bit_colors : 1;
+ unsigned has_con_broken_csi3j : 1;
+ unsigned has_con_broken_il_dl : 1;
+ unsigned has_con_esc_rep : 1;
+ unsigned has_extended_mem_api : 1;
+ unsigned has_tcp_fastopen : 1;
+ unsigned has_linux_tcp_keepalive_sockopts : 1;
+ unsigned has_tcp_maxrtms : 1;
};
};
@@ -98,6 +99,7 @@ public:
bool IMPLEMENT (has_unbiased_interrupt_time)
bool IMPLEMENT (has_precise_interrupt_time)
bool IMPLEMENT (has_posix_unlink_semantics)
+ bool IMPLEMENT (has_posix_unlink_semantics_with_ignore_readonly)
bool IMPLEMENT (has_case_sensitive_dirs)
bool IMPLEMENT (has_posix_rename_semantics)
bool IMPLEMENT (no_msv1_0_s4u_logon_in_wow64)
--
2.30.0
next prev parent reply other threads:[~2021-01-22 15:47 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-15 13:45 [PATCH 00/11] Improve rm speed Ben Wijen
2021-01-15 13:45 ` [PATCH 01/11] syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE first Ben Wijen
2021-01-18 10:25 ` Corinna Vinschen
2021-01-18 10:45 ` Corinna Vinschen
2021-01-18 12:11 ` Ben
2021-01-18 12:22 ` Corinna Vinschen
2021-01-18 14:30 ` Ben
2021-01-18 14:39 ` Corinna Vinschen
2021-01-18 14:59 ` Ben
2021-01-15 13:45 ` [PATCH 02/11] syscalls.cc: Deduplicate _remove_r Ben Wijen
2021-01-18 10:56 ` Corinna Vinschen
2021-01-18 12:40 ` Ben
2021-01-18 13:04 ` Corinna Vinschen
2021-01-18 13:51 ` Ben
2021-01-18 14:49 ` Corinna Vinschen
2021-01-18 14:58 ` Ben
2021-01-15 13:45 ` [PATCH 03/11] syscalls.cc: Fix num_links Ben Wijen
2021-01-18 11:01 ` Corinna Vinschen
2021-01-15 13:45 ` [PATCH 04/11] syscalls.cc: Use EISDIR Ben Wijen
2021-01-18 11:06 ` Corinna Vinschen
2021-01-15 13:45 ` [PATCH 05/11] Cygwin: Move post-dir unlink check Ben Wijen
2021-01-18 11:08 ` Corinna Vinschen
2021-01-18 14:31 ` Ben
2021-01-18 14:52 ` Corinna Vinschen
2021-01-15 13:45 ` [PATCH 06/11] cxx.cc: Fix dynamic initialization for static local variables Ben Wijen
2021-01-18 11:33 ` Corinna Vinschen
2021-01-15 13:45 ` [PATCH 07/11] syscalls.cc: Implement non-path_conv dependent _unlink_nt Ben Wijen
2021-01-18 10:51 ` Ben
2021-01-15 13:45 ` [PATCH 08/11] path.cc: Allow to skip filesystem checks Ben Wijen
2021-01-18 11:36 ` Corinna Vinschen
2021-01-18 17:15 ` Ben
2021-01-19 9:25 ` Corinna Vinschen
2021-01-15 13:45 ` [PATCH 09/11] mount.cc: Implement poor-man's cache Ben Wijen
2021-01-18 11:51 ` Corinna Vinschen
2021-02-03 11:38 ` Ben
2021-02-04 19:37 ` Corinna Vinschen
2021-01-15 13:45 ` [PATCH 10/11] syscalls.cc: Expose shallow-pathconv unlink_nt Ben Wijen
2021-01-15 13:45 ` [PATCH 11/11] dir.cc: Try unlink_nt first Ben Wijen
2021-01-18 12:13 ` Corinna Vinschen
2021-01-18 17:07 ` Ben
2021-01-18 17:18 ` Ben
2021-01-19 9:54 ` Corinna Vinschen
2021-01-20 16:10 ` [PATCH v2 0/8] Improve rm speed Ben Wijen
2021-01-20 16:10 ` [PATCH v2 1/8] syscalls.cc: unlink_nt: Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE Ben Wijen
2021-01-22 10:52 ` Corinna Vinschen
2021-01-22 15:47 ` Ben Wijen [this message]
2021-01-25 10:30 ` [PATCH v3 " Corinna Vinschen
2021-01-20 16:10 ` [PATCH v2 2/8] syscalls.cc: Deduplicate remove Ben Wijen
2021-01-22 12:22 ` Corinna Vinschen
2021-01-22 15:47 ` [PATCH v3 " Ben Wijen
2021-01-25 18:58 ` Corinna Vinschen
2021-01-20 16:10 ` [PATCH v2 3/8] Cygwin: Move post-dir unlink check Ben Wijen
2021-01-22 12:35 ` Corinna Vinschen
2021-01-20 16:10 ` [PATCH v2 4/8] syscalls.cc: Implement non-path_conv dependent _unlink_nt Ben Wijen
2021-01-26 11:34 ` Corinna Vinschen
2021-02-03 11:03 ` Ben
2021-02-04 19:29 ` Corinna Vinschen
2021-01-20 16:10 ` [PATCH v2 5/8] path.cc: Allow to skip filesystem checks Ben Wijen
2021-01-20 16:10 ` [PATCH v2 6/8] syscalls.cc: Expose shallow-pathconv unlink_nt Ben Wijen
2021-01-26 11:45 ` Corinna Vinschen
2021-01-20 16:10 ` [PATCH v2 7/8] dir.cc: Try unlink_nt first Ben Wijen
2021-01-26 11:46 ` Corinna Vinschen
2021-01-27 9:22 ` Ben
2021-01-20 16:10 ` [PATCH v2 8/8] fhandler_disk_file.cc: Use path_conv's IndexNumber Ben Wijen
2021-01-26 12:15 ` Corinna Vinschen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210122154712.3207-1-ben@wijen.net \
--to=ben@wijen.net \
--cc=cygwin-patches@cygwin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).