Hartmut Birr wrote:
This is not true, and I'm currently trying to find some time to write a driver to prove you otherwise, but thankfully I've noticed that this breaks tcpip! Tcpip returns with STATUS_BUFFER_OVERFLOW. The caller is never notified because you don't set the IO STATUS BLOCK unless there was success.
There are also other differences that I will try to prove to you with a driver:
- On ReactOS if you fail an IRP, have PendingReturned to TRUE but are
either IRP_SYNCHRONOUS_API or FO_SYNCHRONOUS_IO, the UserEvent will get signaled. On Windows, it won't, unless you have a FileObject and a SYNC_API IRP. 2) On ReactOS, if you fail an IRP, have PendingReturned to TRUE but are FO_SYNCHRONOUS_IO and IRP_SYNCHRONOUS_API, and specificed a user event, the file event will get signaled. On Windows, it won't.
And others.
These are all the results of not handling the code paths separately like I did, with the following correct logic:
- IRP SUCCESS or PendingReturned with an ASYNC Operation:
- Write IOSB
- Notifiy File Object Event if FO_SYNC_IO.
- Notify User Event
- Do APC, Completion
- Clean up & free code
This is currently done correctly, but is also being done for SYNC. This is wrong.
I agree with this.
So you agree that we shouldn't do it for SYNC IRP or SYNC FO? Good, at least we're getting somewhere then.
- IRP FAILURE or PendingReturned with a SYNC Operation:
- If PendingReturned with a FileObject, and the IRP is sync:
- Write IOSB.
- If There is a User Event, Signal it
- If there isn't, signal the FileObject Event instead (note,
do NOT signal both) - If PendingReturned with a FileObject, and the IRP is async: - Write FinalStatus in the File Object - Signal the FileObject Event
- Cleanup & free code
I do not completley agree with this. There exist four situations on error:
- IRP is SYNC, FO is SYNC
- IRP is SYNC, FO is not SYNC
If Pending returned, then Write the IOSB, and Signal the User OR File Event. Write IOSB. the SYNCicty of the FO is irrelevant in this path.
- IRP is not SYNC, FO is not SYNC
If Pending returned, then Signal the FileObject Event and write FinalStatus. Once again, the SYNCity of the FO is Irrelevant here, so this also applies if the FO is sync.
- IRP is SYNC, no FO
Nothing. Complete and utter failure. No event will be signaled, no status will be written.
- IRP FAILURE or PendingReturned with a SYNC Operation:
- If PendingReturned (always with and without a FO):
- Write IOSB.
- If There is a User Event, Signal it
- If PendingReturned with a FO:
- If the FO is SYNC or if there is no User Event, Signal the
FO Event - If the IRP is SYNC, Write FinelStatus in the FO Event
- Cleanup & free code
Your code doesn't handle situation four
It shouldn't!
and it doesn't return a status to the caller for situation three.
It does, the file event will be signaled and the final status will be written (NOT the IOSB, but the File OBject final status, which is what the caller should be waiting on).
My code does nearly the same as the success part.
Nearly, yes, but misses these sublle differences.
I'm not sure what we must do for a FO with a completion port. If I understand Nagar's 'File System Internals' correctly, the second stage is execute directly and not as an APC. The completion port is called always.
Second stage is always called through APC except in the following cases: IoRetryIoCompletions, a special internal function running at APC_LEVEL when a pagefault is happening and completion is deadlocking for some reason. IopSynchronousServiceTail, an internal function which handles the return of system sync irps handled by ntoskrnl. The routine is called directly at APC level if DEFERRED_IO_COMPLETION was used by the driver. IopAbortRequest, the Rundown APC for IO Completion.
Completion port is called exactly like I coded it in my newest patch, and I followed the Nagar book
Once again, let me remind you that TCPIP is broken on trunk because your code does not return any sort of status to the caller.
Best regards, Alex Ionescu