Re: 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