microsoft / win32metadata

Tooling to generate metadata for Win32 APIs in the Windows SDK.
Other
1.34k stars 119 forks source link

LoadIconWithScaleDown: hinst parameted can be NULL #1932

Closed canton7 closed 5 months ago

canton7 commented 5 months ago

Actual behavior

The generated code for LoadIconWithScaleDown is:

internal static unsafe winmdroot.Foundation.HRESULT LoadIconWithScaleDown(SafeHandle hinst, string pszName, int cx, int cy, out DestroyIconSafeHandle phico)
{
    bool hinstAddRef = false;
    try
    {
        fixed (char* pszNameLocal = pszName)
        {
            winmdroot.Foundation.HINSTANCE hinstLocal;
            if (hinst is object)
            {
                hinst.DangerousAddRef(ref hinstAddRef);
                hinstLocal = (winmdroot.Foundation.HINSTANCE)hinst.DangerousGetHandle();
            }
            else
                throw new ArgumentNullException(nameof(hinst));
            winmdroot.UI.WindowsAndMessaging.HICON phicoLocal;
            winmdroot.Foundation.HRESULT __result = PInvoke.LoadIconWithScaleDown(hinstLocal, pszNameLocal, cx, cy, &phicoLocal);
            phico = new DestroyIconSafeHandle(phicoLocal, ownsHandle: true);
            return __result;
        }
    }
    finally
    {
        if (hinstAddRef)
            hinst.DangerousRelease();
    }
}

However, the documentation states that the hinst parameter can be null:

A handle to the module of either a DLL or executable (.exe) file that contains the icon to be loaded. For more information, see GetModuleHandle.

To load a predefined system icon or a standalone icon file, set this parameter to NULL.

Expected behavior

The generated LoadIconWithScaleDown allows hinst to be null.

Repro steps

  1. NativeMethods.txt content:

    LoadIconWithScaleDown
  2. NativeMethods.json content (if present):

  3. None.

  4. Any of your own code that should be shared?

None

Context

canton7 commented 5 months ago

It looks like the same issues applies to LoadIconMetric as well

AArnott commented 5 months ago

Thanks for the feedback. This is a shortcoming of the metadata, which does not annotate these parameters as optional. I'll move this issue to the metadata repo so it can be corrected.