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.
>
>> 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?
Yes, the existing fallback, should be sufficient.
>
>>
>> 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 :)
Ok, will do.
>
>
> Corinna
>
