Re: [PATCH v10 0/5] MR9045: windows.devices.enumeration: Add support for device properties in DeviceInformation objects.

[Rémi Bernon (＠rbernon)]

Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/main.c:
> -
>    const WCHAR *buf;
> -
>    HSTRING name;
> -
> -
>    if (FAILED(hr = IIterator_HSTRING_get_Current( names, &name ))) break;
> -
>    buf = WindowsGetStringRawBuffer( name, NULL );
> -
>    if (buf[0] == '{')
> -
>        hr = PSPropertyKeyFromString( buf, (PROPERTYKEY *)&key.Key );
> -
>    else
> -
>        hr = PSGetPropertyKeyFromName( buf, (PROPERTYKEY *)&key.Key );
> -
>    WindowsDeleteString( name );
> -
>    if (FAILED(hr)) break;
> -
>    /* DevGetObjects(Ex) will not de-duplicate properties, so we need to do it ourselves. */
> -
>    if (!devpropcompkey_buf_find_devpropkey( *keys, *keys_len, key.Key ))
> -
>    {
> -
>        (*keys)[*keys_len] = key;
> -
>        *keys_len += 1;

Fwiw the reason I did it with a local variable is because as a general rule I think it's better to avoid accessing/writing to pointers within a loop. I also find it more readable to have the results of a computation only written at some specific sucesss/failure points rather than being spread around, and it could be arguably incorrect to return a failure while still having modified the array. Shouldn't matter much here anyway.