microsoft / win32metadata

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

Bug: All methods in IEnumShellItems should not return void but HRESULT #1953

Closed 0x5bfa closed 3 months ago

0x5bfa commented 4 months ago

IEnumShellItems has 4 methods but CsWin32 produces as below (deleted comments):

[Guid("70629033-E363-4A28-A567-0DB78006E6D7"),InterfaceType(ComInterfaceType.InterfaceIsIUnknown),ComImport()]
[SupportedOSPlatform("windows6.0.6000")]
[global::System.CodeDom.Compiler.GeneratedCode("Microsoft.Windows.CsWin32", "0.3.106+a37a0b4b70")]
public interface IEnumShellItems
{
    unsafe void Next(uint celt, [Out] [MarshalAs(UnmanagedType.LPArray,SizeParamIndex = 0)] winmdroot.UI.Shell.IShellItem[] rgelt, [Optional] uint* pceltFetched);

    void Skip(uint celt);

    void Reset();

    void Clone(out winmdroot.UI.Shell.IEnumShellItems ppenum);
}

But definitions are:

IEnumShellItems::Clone Gets a copy of the current enumeration.

IEnumShellItems::Next Gets an array of one or more IShellItem interfaces from the enumeration.

IEnumShellItems::Reset Resets the internal count of retrieved IShellItem interfaces in the enumeration.

IEnumShellItems::Skip Skips a given number of IShellItem interfaces in the enumeration. Used when retrieving interfaces.


I cannot remember where but I think I found this issue another function. Is there a config to change to HRESULT or does this something to do with CsWin32 for example it makes functions return void if return value type is unknown?

oold commented 3 months ago

https://github.com/microsoft/CsWin32?tab=readme-ov-file#customizing-generated-code

0x5bfa commented 3 months ago

What do you mean? This wrong generated code is only when you set allowMarshaling=true I want to use it but I obliged to set it false. I know other people posted Discussion in CsWin32 tho, I wanted to report this here as well.

riverar commented 3 months ago

I believe @oold is pointing you to CsWin32's PreserveSig flag.

Example: https://github.com/microsoft/CsWin32/blob/main/test/GenerationSandbox.Tests/NativeMethods.json

riverar commented 3 months ago

Not seeing a metadata bug here, so closing for now. But feel free to keep the discussion going!

oold commented 3 months ago

Correct. I wanted to hint at PreserveSig. The HRESULT-returning methods actually returning void is intentional. An exception is thrown if the result was an error. You need to set preserveSigMethods in the NativeMethods.json file if you want to check the result manually.

Specific advice regarding the IEnumShellItems interface: I presume you want to check whether Next() returned S_FALSE. Good news! You don't have to. The returned number of fetched items will be 0 if there are no further items.

0x5bfa commented 3 months ago

I actually have another issue with allowMarshal where NativeAOT isn't compatible with this option set true. And so I can't use that option.

https://github.com/files-community/Files/pull/16016

But that is cool! Thank you for pointing it out:)