Was: [ros-dev] Re: [ros-svn] [hbirr] 14988: - Guard the copying to the IOSB.

[Alex Ionescu]

Hartmut Birr wrote:

>Copied from the archive because I will receive the mail in some hours:
>  
>
>>hbirr at svn.reactos.com wrote:
>>    
>>
>
>  
>
>>>- Guard the copying to the IOSB.
>>>- Do the main processing on success or if previous STATUS_PENDING was
>>>      
>>>
>returned.
>  
>
>>>Do not use some of the IRP and FO flags at this point.
>>>
>>>
>>>      
>>>
>>Ok, I was going to complain about this, but I realize that it must be
>>used since defintely some parts of ROS don't work properly with those
>>flags set.
>>    
>>
>
>It isn't possible to use the flags, because some functions are always
>synchronous independently which flags are set. Ntoskrnl may set the
>flags correctly, but a driver may possible not.
>  
>
It's not our job to make 3rd-party drivers happy. If they work on 
Windows, which uses the flags, then they must work on ReactOS. If our 
drivers are broken, then they must be fixed.

>  
>
>>>- Set all results before signaling the events.
>>>- Signal the FO event previous the user event.
>>>- Made the code a little bit shorter.
>>>
>>>
>>>
>>>      
>>>
>>I like your changes and have no complaints, except that the signaling
>>semantics of the File/User events should be different in the failure
>>case (my "else" path). You've completely removed that path and thus
>>changed the logic.
>>    
>>
>
>If the driver has returned STATUS_PENDING and completes the irp later
>with an error, both events must be signaled, because two different
>threads may waiting on this events. The IOSB must be also set. The apc
>must be also deliver (I've tested this with the apc sample and some
>modifications on windows). There is no difference between completing the
>irp with and without an error if the driver has returned STATUS_PENDING.
>  
>
Your code now says "    if (NT_SUCCESS(Irp->IoStatus.Status) || 
Irp->PendingReturned)". Not only is this incorrect because the flag must 
be checked, but my code had a path which handled this being FALSE. 
Currently ros does not have this path anymore after your changes. So if 
IoStatus.Status is a failure and PendingReturned is not set, nothing 
will be done. In my version of the code (which is what NT does), the 
events were still signaled in a specific case (see old code).

Best regards,
Alex Ionescu