Re: [PATCH 3/4] include: Define IoSkipCurrentIrpStackLocation

[Aric Stewart]

On 10/12/16 4:10 PM, Sebastian Lackner wrote:
> On 12.10.2016 12:31, Aric Stewart wrote:
> > Signed-off-by: Aric Stewart
> >
> > include/ddk/wdm.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > 0003-include-Define-IoSkipCurrentIrpStackLocation.txt
> >
> > diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h
> > index 5602f7d..5671948 100644
> > --- a/include/ddk/wdm.h
> > +++ b/include/ddk/wdm.h
> > @@ -1175,9 +1175,11 @@ NTSTATUS WINAPI ObCloseHandle(IN HANDLE handle);
> >  # ifdef NONAMELESSSTRUCT
> >  #  define IoGetCurrentIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.s.u2.CurrentStackLocation)
> >  #  define IoGetNextIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.s.u2.CurrentStackLocation - 1)
> > +#  define IoSkipCurrentIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.s.u2.CurrentStackLocation++); ((_Irp)->CurrentLocation++)
> >  # else
> >  #  define IoGetCurrentIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.u2.CurrentStackLocation)
> >  #  define IoGetNextIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.u2.CurrentStackLocation - 1)
> > +#  define IoSkipCurrentIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.u2.CurrentStackLocation++); ((_Irp)->CurrentLocation++)
> >  # endif
> >  #else
> >  # ifdef NONAMELESSSTRUCT
>
> Is this definition based on a Windows header file? I have only found two versions,
> either with a "{ ... }" block or with a static inline function. Your definition
> would not work correctly in a situation like this:
>
> if (...) IoSkipCurrentIrpStackLocation(irp);
>
> Which would be expanded to:
>
> if (...) irp->Tail.Overlay.s.u2.CurrentStackLocation++;
> irp->CurrentLocation++;
>
> Please note that the second statement is no longer conditional. Also, what about
> the two other #ifdef branches directly below?

True, the wdm.h I looked at in the ddk did not appear to block the two statements together so your statement would not work on windows either. I was adding them to look like the other statements in our header.

Now I have not opposition to putting them in a block, but just saying that it is not done that way in the windows header.

Yeah, i just missed the other 2 locations. But I should update them with the correct bracketing.

The rest of the code seems to use (...) but do you feel like it should be {..}?

-aric
> Regards,
> Sebastian