Re: [PATCH 5/5] winebus.sys: Use irp IoStatus.Status consistently.

[Zebediah Figura (she/her)]

On 8/12/21 2:47 AM, Rémi Bernon wrote:
>       case IOCTL_HID_READ_REPORT:
>       {
>           TRACE_(hid_report)("IOCTL_HID_READ_REPORT\n");
> -
>        status = ext->vtbl->begin_report_processing(device);
> -
>        if (status != STATUS_SUCCESS)
> -
>        {
> -
>            irp->IoStatus.Status = status;
> -
>            break;
> -
>        }
> -
>        irp->IoStatus.Status = ext->vtbl->begin_report_processing(device);
> -
>        if (irp->IoStatus.Status != STATUS_SUCCESS) break;
>         if (!ext->last_report_read)
>         {
> -
>            irp->IoStatus.Status = status = deliver_last_report(ext,
> -
>            irp->IoStatus.Status = deliver_last_report(ext,
>                 buffer_len, irp->UserBuffer, &irp->IoStatus.Information);
>             ext->last_report_read = TRUE;
>         }
>         else
>         {
>             InsertTailList(&ext->irp_queue, &irp->Tail.Overlay.ListEntry);
> -
>            status = STATUS_PENDING;
> -
>            irp->IoStatus.Status = STATUS_PENDING;
>         }
>         break;
>     }

If you return STATUS_PENDING and queue the IRP here...
> @@ -990,9 +982,8 @@ static NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp)
>   LeaveCriticalSection(&ext->cs);

...and release the CS here, then the IRP can be completed before you 
access it...
> - if (status != STATUS_PENDING)
> -
>    IoCompleteRequest(irp, IO_NO_INCREMENT);
> -
> - status = irp->IoStatus.Status;

...here.

This is kind of the reason that I've gone for the pattern

switch (ioctl)
     {
         case X:
             status = STATUS_SUCCESS;
             break;

case Y:
             status = STATUS_PENDING;
             IoMarkIrpPending(irp);
             queue_irp(irp);
             break;
     }

if (status != STATUS_PENDING)
     {
         irp->IoStatus.Status = status;
         IoCompleteRequest(irp);
     }
     return status;

in other ntoskrnl code; it's almost the same amount of code and less 
easy to get wrong.
> - if (status != STATUS_PENDING) IoCompleteRequest(irp, IO_NO_INCREMENT);
>  return status;
> }