[PATCH 3/4] setupapi: Create only one driver per driver id match.

[Zebediah Figura (she/her)]

On 8/17/21 9:39 AM, Rémi Bernon wrote:
> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
> ---
>   dlls/setupapi/devinst.c       | 26 +++++++++++++++-----------
>   dlls/setupapi/tests/devinst.c |  9 +--------
>   2 files changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c
> index baef28ac130..13e52f4534a 100644
> --- a/dlls/setupapi/devinst.c
> +++ b/dlls/setupapi/devinst.c
> @@ -4648,6 +4648,7 @@ static void enum_compat_drivers_from_file(struct device *device, const WCHAR *pa
>       DWORD i, j, k, driver_count = device->driver_count;
>       struct driver driver, *drivers = device->drivers;
>       INFCONTEXT ctx;
> +    BOOL found;
>       HINF hinf;
>   
>       TRACE("Enumerating drivers from %s.\n", debugstr_w(path));
> @@ -4687,20 +4688,23 @@ static void enum_compat_drivers_from_file(struct device *device, const WCHAR *pa
>   
>           for (j = 0; SetupGetLineByIndexW(hinf, driver.mfg_key, j, &ctx); ++j)
>           {
> -            for (k = 2; SetupGetStringFieldW(&ctx, k, id, ARRAY_SIZE(id), NULL); ++k)
> +            for (k = 2, found = FALSE; SetupGetStringFieldW(&ctx, k, id, ARRAY_SIZE(id), NULL); ++k)
>               {
> -                if (device_matches_id(device, HardwareId, id) || device_matches_id(device, CompatibleIDs, id))
> -                {
> -                    SetupGetStringFieldW(&ctx, 0, driver.description, ARRAY_SIZE(driver.description), NULL);
> -                    SetupGetStringFieldW(&ctx, 1, driver.section, ARRAY_SIZE(driver.section), NULL);
> +                if ((found = device_matches_id(device, HardwareId, id))) break;
> +                if ((found = device_matches_id(device, CompatibleIDs, id))) break;
> +            }
>   
> -                    TRACE("Found compatible driver: manufacturer %s, desc %s.\n",
> -                            debugstr_w(driver.manufacturer), debugstr_w(driver.description));
> +            if (found)
> +            {
> +                SetupGetStringFieldW(&ctx, 0, driver.description, ARRAY_SIZE(driver.description), NULL);
> +                SetupGetStringFieldW(&ctx, 1, driver.section, ARRAY_SIZE(driver.section), NULL);
>   
> -                    driver_count++;
> -                    drivers = heap_realloc(drivers, driver_count * sizeof(*drivers));
> -                    drivers[driver_count - 1] = driver;
> -                }
> +                TRACE("Found compatible driver: manufacturer %s, desc %s.\n",
> +                        debugstr_w(driver.manufacturer), debugstr_w(driver.description));
> +
> +                driver_count++;
> +                drivers = heap_realloc(drivers, driver_count * sizeof(*drivers));
> +                drivers[driver_count - 1] = driver;
>               }
>           }
>       }

I must be missing something; wouldn't it be simpler and functionally 
equivalent just to put a "break" at the end of the inner "if"?