Hello Jacek,

    thanks for the review.

On 2/20/19 6:33 PM, Paul Gofman wrote:

 -    if (send_completion) NTDLL_AddCompletion( hFile, cvalue, status, total );
-    if (async_read && (options & FILE_NO_INTERMEDIATE_BUFFERING) && status == STATUS_SUCCESS)
-        return STATUS_PENDING;
-    return status;
+    ret_status = async_read && (options & FILE_NO_INTERMEDIATE_BUFFERING) && status == STATUS_SUCCESS
+            ? STATUS_PENDING : status;
+
+    if (send_completion) NTDLL_AddCompletion( hFile, cvalue, status, total, ret_status == STATUS_PENDING );
+    return ret_status;
 }

Do you really need a separated ret_status variable? It seems that you could just set status to STATUS_PENDING instead.

This way I will be passing STATUS_PENDING instead of STATUS_SUCCESS to NTDLL_AddCompletion() status parameter. This looks like a separate change which I tried to avoid. Or do you think it is ok to do that?

 
 
@@ -1089,7 +1090,7 @@ NTSTATUS WINAPI NtReadFileScatter( HANDLE file, HANDLE event, PIO_APC_ROUTINE ap
     if (event) NtSetEvent( event, NULL );
     if (apc) NtQueueApcThread( GetCurrentThread(), (PNTAPCFUNC)apc,
                                (ULONG_PTR)apc_user, (ULONG_PTR)io_status, 0 );
-    if (send_completion) NTDLL_AddCompletion( file, cvalue, status, total );
+    if (send_completion) NTDLL_AddCompletion( file, cvalue, status, total, TRUE );
 
     return STATUS_PENDING;
 
@@ -1408,7 +1409,7 @@ err:
         if (status != STATUS_PENDING && hEvent) NtResetEvent( hEvent, NULL );
     }
 
-    if (send_completion) NTDLL_AddCompletion( hFile, cvalue, status, total );
+    if (send_completion) NTDLL_AddCompletion( hFile, cvalue, status, total, status == STATUS_PENDING );

Unless I'm missing, send_completion will never be set if status == STATUS_PENDING in this case, so you could just pass FALSE.

Yeah, looks so, I will change that.