[2/10]hidclass.sys: add hidclass.sys

[Henri Verbeet]

I didn't really look at the rest of the series, but here are a few
things that caught my eye.

On 6 July 2015 at 17:46, Aric Stewart <aric at codeweavers.com> wrote:
> +struct list minidriver_list = LIST_INIT(minidriver_list);
You don't seem to access this outside of main.c anywhere in this
series, and it seems somewhat unlikely that you'll ever have to.
Shouldn't this just be static?

> +static VOID WINAPI UnloadDriver(DRIVER_OBJECT *driver)
"void"

> +    TRACE("Driver Unload\n");
> +    LIST_FOR_EACH_ENTRY_SAFE(md, next, &minidriver_list, minidriver, entry)
> +    {
> +        if (md->minidriver.DriverObject == driver)
> +        {
> +            if (md->DriverUnload)
> +                md->DriverUnload(md->minidriver.DriverObject);
> +            list_remove(&md->entry);
> +            break;
> +        }
> +    }
> +}
Did you intend to use find_minidriver() here?

> +minidriver* find_minidriver(DRIVER_OBJECT* driver)
> +{
> +    minidriver *md;
> +    LIST_FOR_EACH_ENTRY(md, &minidriver_list, minidriver, entry)
> +    {
> +        if (md->minidriver.DriverObject == driver)
> +            return md;
> +    }
> +    return NULL;
> +}
As the code currently is this isn't actually used until patch 10/10.
You could potentially use it from UnloadDriver() as mentioned above,
but it should still initially be static in that case. Your '*'
placement is inconsistent.

> +NTSTATUS WINAPI HidRegisterMinidriver(PHID_MINIDRIVER_REGISTRATION registration)
"HID_MINIDRIVER_REGISTRATION *registration"

> +BOOL WINAPI DllMain(HINSTANCE hInstDLL, DWORD fdwReason, LPVOID lpv)
> +{
> +    switch(fdwReason)
> +    {
> +        case DLL_PROCESS_ATTACH:
> +            DisableThreadLibraryCalls(hInstDLL);
> +            break;
> +    }
> +    return TRUE;
> +}
As long as you don't do anything special in here, you don't need an
explicit DllMain(). (But if you did, "void *" instead of LPVOID, and
parameter naming.)

> +/* These are from hidport.h but are only used here anyway */
> +typedef struct _HID_MINIDRIVER_REGISTRATION {
The brace placement is inconsistent with the rest of the code.

> +    ULONG           Revision;
> +    PDRIVER_OBJECT  DriverObject;
> +    PUNICODE_STRING RegistryPath;
"UNICODE_STRING *RegistryPath;"

> +    ULONG           DeviceExtensionSize;
> +    BOOLEAN         DevicesArePolled;
> +    UCHAR           Reserved[3];
> +} HID_MINIDRIVER_REGISTRATION, *PHID_MINIDRIVER_REGISTRATION;
If this is really only used inside hidclass.sys, you shouldn't need
the PHID_MINIDRIVER_REGISTRATION typedef.

> +NTSTATUS WINAPI HidRegisterMinidriver(PHID_MINIDRIVER_REGISTRATION  MinidriverRegistration);
The whitespace is odd here. Also, "HID_MINIDRIVER_REGISTRATION *registration".

> +typedef struct _minidriver
> +{
> +    struct list entry;
> +
> +    HID_MINIDRIVER_REGISTRATION minidriver;
> +
> +    PDRIVER_UNLOAD DriverUnload;
> +} minidriver;
I suppose it's somewhat up to personal style, but you don't need a
typedef for this. Aside from saving you from typing "struct" a few
times it only obfuscates the code.

I suspect some of the imports and includes aren't actually used until
later in the series, but I didn't explicitly check that.