Re: [PATCH v2 0/5] MR551: win32u: Complete support for display modes in nulldrv.

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
>  if ((!devname || !devname->Length) && !devmode)
>  {
>      ret = user_driver->pChangeDisplaySettingsEx( NULL, NULL, hwnd, flags, lparam );
> -
>    if (ret != DISP_CHANGE_SUCCESSFUL)
> -
>        ERR( "Restoring all displays to their registry settings returned %d.\n", ret );
> -
>    if (ret != E_NOTIMPL)
> -
>    {
> -
>        if (ret) ERR( "Restoring all displays to their registry settings returned %d.\n", ret );
> -
>        return ret;
> -
>    }
> -
> -
>    for (i = 0; !NtUserEnumDisplayDevices( NULL, i, &info, 0 ); ++i)
> -
>    {
> -
>        UNICODE_STRING str;
> -
>        RtlInitUnicodeString( &str, info.DeviceName );
> -
>        if ((ret = NtUserChangeDisplaySettings( &str, NULL, hwnd, flags, lparam ))) break;

I don't like the way the nulldrv display settings handler code is mixing with the normal code path. You should place nulldrv's ChangeDisplaySettings implementation in nulldrv_ChangeDisplaySettingsEx(), even at the expense of code duplication. Same for the force parameter in update_display_cache(). There should be a nulldrv_UpdateDisplayDevices that always succeeds. Then you don't need to check E_NOTIMPL everywhere. Same for below.