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

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

Hi Ben,

after venting that I missed this flag, back to the patch itself:

On Jan 15 14:45, Ben Wijen wrote:
> ---
>  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
> @@ -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 525efecf3..ce4e9c65c 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -709,11 +709,23 @@ _unlink_nt (path_conv &pc, 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???
> +      /* Try FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE first
> +         it was added with Redstone 5 (Win10 18_09) (as were POSIX rename semantics)
> +         If it fails: fall-back to usual trickery */
> +      if (wincap.has_posix_rename_semantics ())
> +        {
> +          fdie.Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS;
> +          fdie.Flags|= FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE;
> +          status = NtSetInformationFile (fh, &io, &fdie, sizeof fdie,
> +                                         FileDispositionInformationEx);
> +          if (NT_SUCCESS (status))
> +            {
> +              NtClose (fh);
> +              goto out;
> +            }
> +        }

Rather than calling NtSetInformationFile here again, we should rather
just skip the transaction stuff on 1809 and later.  I'd suggest adding
another wincap flag like, say, "has_posix_ro_override", being true
for 1809 and later.  Then we can skip the transaction handling if
wincap.has_posix_ro_override () and just add the
FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE flag to fdie.Flags, if
it's available.


Thanks,
Corinna