[PATCH v3 4/5] ntdll: Return STATUS_PENDING from NtWriteFile() for async write.

[Paul Gofman]

On 3/4/19 20:10, Jacek Caban wrote:
> On 3/4/19 3:03 PM, Paul Gofman wrote:
>> Hello Jacek,
>>
>>     so far I grepped the source tree for the WriteFile. This was 
>> supposed to show me WriteFileEx and NtWriteFile calls too. If 
>> LPOVERLAPPED (the last parameter to WriteFile) is NULL, WriteFile is 
>> going to fail with async file handles (at least for regular / serial 
>> / device files) regardless of actual sync or async.
>
>
> Why would WriteFile with NULL overlapped fail? It will wait on 
> provided handle, which is suboptimal, but it should work. Anyway, it's 
> fine to skip those cases in your case.
>
While kernel32.WriteFile() is ok with NULL overlapped structure and is 
ready to wait, NtWriteFile() is not: it will get 'PLARGE_INTEGER offset' 
parameter as NULL from kernel32.WriteFile, and will fail with 
STATUS_INVALID_PARAMETER for async files (at least regular, serial or 
device files). I also just tested that on a minimal example which 
attempts to call kernel32.WriteFile() without overlapped structure, this 
fails for me for file created with FILE_FLAG_OVERLAPPED both under Wine 
and Windows 7.

>
>
>
>> Do you think it makes sense if I proceed to change ntdll tests as 
>> described before (marking pre-Vista behaviour as invalid) and then 
>> resend the patch concerning NtWriteFile() with the async return for 
>> regular files only? 
>
>
> Mostly yes. I'm not sure about doing this only for regular files, 
> through. I know it addresses my concern, but your testing seems to 
> indicate that it's pipes that are unusual and, as far as I understand, 
> all other tested types return STATUS_PENDING. If that's the case, 
> maybe we should return STATUS_PENDING without depending on handle type?
>
My testing covered regular files so far. The change (in its initial 
form) can pass the existing tests for special files (as STATUS_PENDING 
return is surely possible for the other types), but I don't think we 
have enough tests yet to be sure it is always async for all the file 
types. Besides pipes (for which we probably should avoid always 
returning async, even if NtReadFile() is not used in Wine for them?), I 
would expect sync return to be possible at least for sockets. It is also 
hard to be sure whether we really can never get sync result from a 
special file, or just did not reproduce the right case. I also thought 
that in any case, even if sockets and all the other types turn out to 
work async always, it is safer to change the behaviour in smaller parts 
for more precise bisect in case of regressions (yes, my initial patch 
did it for all file types together, but I tend to treat that as a very 
unfortunate overlook from my side).

Anyway, if you feel like it is better to cover that in respect to all 
the possible file types, I can start thinking of adding fd type specific 
tests. In this case, do you think we should try bringing all the tests 
directly to ntdll/tests, or rather extend the tests existing elsewhere 
(e. g. in ws2_32 for sockets)? I would probably stick to the latter 
unless there are reasons not to.