Re: [PATCH v2 4/4] winehid: Implement handing internal ioctls

[Sebastian Lackner]

On 12.10.2016 18:19, Aric Stewart wrote:
> Mostly by passing them to the bus driver where platform specific actions
> happen
>
> v2: Suggestion from Sebastian Lackner
> Signed-off-by: Aric Stewart <aric(a)codeweavers.com>
> ---
>  dlls/winehid.sys/main.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)

This patch no longer applies because of the slightly modified version of
patch 1/2. I have already prepared a rebased version, but I'm wondering a bit:
Couldn't we just pass through ALL ioctls (without any filtering in winehid)?
This would reduce the code complexity even further. HID related FIXMEs could
also be printed in winebus. hid_internal_dispatch should probably use the
"hid" debug channel anyway. What do you think?

Regards,
Sebastian

[PATCH v2 4/4] winehid: Implement handing internal ioctls

[Aric Stewart]

On 10/14/16 5:09 AM, Sebastian Lackner wrote:
> On 12.10.2016 18:19, Aric Stewart wrote:
> > Mostly by passing them to the bus driver where platform specific actions
> > happen
> >
> > v2: Suggestion from Sebastian Lackner
> > Signed-off-by: Aric Stewart <aric(a)codeweavers.com>
> > ---
> >  dlls/winehid.sys/main.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
>
> This patch no longer applies because of the slightly modified version of
> patch 1/2. I have already prepared a rebased version, but I'm wondering a bit:
> Couldn't we just pass through ALL ioctls (without any filtering in winehid)?
> This would reduce the code complexity even further. HID related FIXMEs could
> also be printed in winebus. hid_internal_dispatch should probably use the
> "hid" debug channel anyway. What do you think?
>
> Regards,
> Sebastian

Yup, rebased and resent

thanks!
-aric

[PATCH v2 4/4] winehid: Implement handing internal ioctls

[Sebastian Lackner]

On 14.10.2016 06:25, Aric Stewart wrote:
> Yup, rebased and resent
>
> thanks!
> -aric

Actually I was thinking about an even more simplified solution like:

--- snip ---
static NTSTATUS WINAPI internal_ioctl(DEVICE_OBJECT *device, IRP *irp)
{
    /* All these are handled by the lower level driver */
    IoSkipCurrentIrpStackLocation(irp);
    return IoCallDriver(((HID_DEVICE_EXTENSION *)device->DeviceExtension)->NextDeviceObject, irp);
}
--- snip ---

The function in winebus also has tracing for unsupported ioctls for example.
Does that make sense, or do you prefer to keep the IoControlCode checks
(even if we do not seem to need them yet).

Regards,
Sebastian

[PATCH v2 4/4] winehid: Implement handing internal ioctls

[Aric Stewart]

On 10/14/16 6:35 AM, Sebastian Lackner wrote:
> On 14.10.2016 06:25, Aric Stewart wrote:
> > Yup, rebased and resent
> >
> > thanks!
> > -aric
>
> Actually I was thinking about an even more simplified solution like:
>
> --- snip ---
> static NTSTATUS WINAPI internal_ioctl(DEVICE_OBJECT *device, IRP *irp)
> {
>     /* All these are handled by the lower level driver */
>     IoSkipCurrentIrpStackLocation(irp);
>     return IoCallDriver(((HID_DEVICE_EXTENSION *)device->DeviceExtension)->NextDeviceObject, irp);
> }
> --- snip ---
>
> The function in winebus also has tracing for unsupported ioctls for example.
> Does that make sense, or do you prefer to keep the IoControlCode checks
> (even if we do not seem to need them yet).
>
> Regards,
> Sebastian

I think I prefer keeping the IoControlCode checks and having an explicit passthrough of only the HID things for now. Thought I realized you had removed the ntoskrnl dependency so my v3 patch is incorrect. Working a v4.

-aric