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

Corinna Vinschen corinna-cygwin@cygwin.com
Mon Jan 18 12:22:11 GMT 2021


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.


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

> So a retry without FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE is valid here.

That would be a border case which might then occur with the
FILE_DISPOSITION_POSIX_SEMANTICS flag, too.  The current code falls
through anyway, that should be sufficient, right?

> 
> I have thought about adding wincap.has_posix_unlink_semantics_with_ignore_readonly
> but it is equal to wincap.has_posix_rename_semantics so I didn't bother adding it.

It doesn't hurt to add another flag with the same values.  It's better
readable in context, which easily makes up for the extra bit :)


Corinna


More information about the Cygwin-patches mailing list