Re: [PATCH] HID: Handle failure to create device by trying to open it instead

[Huw Davies]

On Tue, Mar 19, 2019 at 01:44:56PM -0500, Aric Stewart wrote:
> Signed-off-by: Aric Stewart
>
> dlls/hidclass.sys/device.c |  9 +++++----
>  dlls/winebus.sys/main.c    | 15 ++++++++++++---
>  2 files changed, 17 insertions(+), 7 deletions(-)
> diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c
> index 547c355410..55bcb66eb3 100644
> --- a/dlls/hidclass.sys/device.c
> +++ b/dlls/hidclass.sys/device.c
> @@ -99,13 +99,14 @@ NTSTATUS HID_LinkDevice(DEVICE_OBJECT *device)
>      Data.cbSize = sizeof(Data);
>      if (!SetupDiCreateDeviceInfoW(devinfo, ext->instance_id, &GUID_DEVCLASS_HIDCLASS, NULL, NULL, DICD_INHERIT_CLASSDRVS, &Data))
>      {
> -
>    BOOL rc = FALSE;
>    if (GetLastError() == ERROR_DEVINST_ALREADY_EXISTS)
> -
>        rc = SetupDiOpenDeviceInfoW(devinfo, ext->instance_id, NULL, DICD_INHERIT_CLASSDRVS, &Data);
> -
>    if (!rc)
>    {
> -
>        SetupDiDestroyDeviceInfoList(devinfo);
> -
>        return STATUS_SUCCESS;
> -
>        FIXME( "failed to Create Device Info %x\n", GetLastError());
> -
>        goto error;
>    }
> -
>    FIXME( "failed to Create Device Info %x\n", GetLastError());
> -
>    goto error;
> }

This looks a bit clumsy to me.  How about moving BOOL rc to function scope and then
something like:

rc = SetupDiCreateDeviceInfoW(devinfo, ext->instance_id, &GUID_DEVCLASS_HIDCLASS, NULL, NULL, DICD_INHERIT_CLASSDRVS, &Data))
    if (!rc && GetLastError() == ERROR_DEVINST_ALREADY_EXISTS)
        rc = SetupDiOpenDeviceInfoW(devinfo, ext->instance_id, NULL, DICD_INHERIT_CLASSDRVS, &Data);
    if (!rc)
    {
        FIXME( "failed to Create Device Info %x\n", GetLastError());
        goto error;
    }
>  if (!SetupDiRegisterDeviceInfo( devinfo, &Data, 0, NULL, NULL, NULL ))
>  {
>
> diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c
> index fd68833ac3..779ba34dfb 100644
> --- a/dlls/winebus.sys/main.c
> +++ b/dlls/winebus.sys/main.c
> @@ -263,13 +263,22 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW,
>      {
>          SP_DEVINFO_DATA data;
>          WCHAR *instance;
> -
>    BOOL rc = FALSE;

No need for the initialization.
>      data.cbSize = sizeof(data);
>      if (!(instance = get_instance_id(device)))
>          ERR("failed to generate instance id\n");
> -
>    else if (!SetupDiCreateDeviceInfoW(devinfo, instance, class, NULL, NULL, DICD_INHERIT_CLASSDRVS, &data))
> -
>        ERR("failed to create device info: %x\n", GetLastError());
> -
>    else if (!SetupDiRegisterDeviceInfo(devinfo, &data, 0, NULL, NULL, NULL))
> -
>    rc = SetupDiCreateDeviceInfoW(devinfo, instance, class, NULL, NULL, DICD_INHERIT_CLASSDRVS, &data);
> -
>    if (!rc)
> -
>    {
> -
>        if (GetLastError() == ERROR_DEVINST_ALREADY_EXISTS)
> -
>            rc = SetupDiOpenDeviceInfoW(devinfo, instance, NULL, DICD_INHERIT_CLASSDRVS, &data);
> -
>        if (!rc)
> -
>        {
> -
>            ERR("failed to open device info: %x\n", GetLastError());
> -
>        }
> -
>    }

And if this was also written as I suggested above it would avoid a level of indentation.

Huw.