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

[Corinna Vinschen]

On Jan 18 15:30, Ben wrote:
> 
> 
> 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 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.
> >>
> >> Hmmm, I'm not sure if I follow you: This extra NtSetInformationFile is not
> >> related to the transaction stuff?
> > 
> > Right, sorry.  I meant the
> > 
> >   if (pc.file_attributes () & FILE_ATTRIBUTE_READONLY)
> > 
> > bracketed code in fact.  What I meant is to keep it at
> > 
> >   open
> >   if (ro)
> >     setattributes
> >   setinformation
> >   ...
> > 
> > and only add the required additional flag.
> 
> Ah, yes I understand. The extra NtSetInformation was there for
> the fallback without FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE
> 
> I have seen bordercases, but I have not seen NtSetInformation fail
> FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE and succeed without.
> Even if it would, Your suggestion does save a bunch of code...
> 
> > 
> > 
> >> Also I have seen NtSetInformationFile fail with STATUS_INVALID_PARAMETER.
> > 
> > That should only occur on pre-1809 then, and adding the extra wincap
> > would fix that.
> 
> Do note: This can also happen post-1809 with a driver that hasn't implemented it yet.

I'm sure, but that code path is called on non-remote ntfs only anyway.


Corinna