Add RAII for PIDL types (BSTR-style) and FreeWith(CoTaskMemFree) for CoTaskMem string out-params - #2276
Draft
antmor wants to merge 4 commits into
Draft
Add RAII for PIDL types (BSTR-style) and FreeWith(CoTaskMemFree) for CoTaskMem string out-params#2276antmor wants to merge 4 commits into
antmor wants to merge 4 commits into
Conversation
…feHandle Model PIDLIST_RELATIVE, PIDLIST_ABSOLUTE, and PITEMID_CHILD as NativeTypedef handles with CloseApi=ILFree in autoTypes.json, following the HLOCAL/HGLOBAL (void* + free-function) precedent. This lets CsWin32 emit an ILFree-based SafeHandle for PIDLs instead of requiring consumers to hand-roll RAII around SHGetIDListFromObject, ILCreateFromPath, SHGetKnownFolderIDList, etc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…or string out-params - PIDL typedefs (PIDLIST_RELATIVE/ABSOLUTE, PITEMID_CHILD) now follow the BSTR pattern: NativeTypedef wrapping the real ITEMIDLIST* pointee (no void*) with CloseApi=CoTaskMemFree, so CsWin32 generates a strongly-typed SafeHandle. Namespace is inherited from the CloseApi (as BSTR/HANDLE do). - Add FreeWith(CoTaskMemFree) annotations for PWSTR/LPOLESTR out-params that must be freed with CoTaskMemFree (the unique_cotaskmem_string cases): SHGetNameFromIDList, StringFromCLSID, StringFromIID, IShellItem::GetDisplayName. Mirrors the existing SHGetKnownFolderPath::ppszPath precedent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Placing the PIDL NativeTypedefs in the CloseApi's inherited namespace put them in System.Com while their ITEMIDLIST* payload lives in Shell.Common, creating a System.Com <-> Shell.Common cycle (NoCyclicalNamespaces failure). Fix keeps the strong ITEMIDLIST* typing by co-locating everything in Shell.Common: - PIDL typedefs (PIDLIST_RELATIVE/ABSOLUTE, PITEMID_CHILD) explicitly in Windows.Win32.UI.Shell.Common with ValueType=ITEMIDLIST* and CloseApi=ILFree. - Move ILFree into Shell.Common via requiredNamespacesForNames.rsp so the CloseApi is intra-namespace (satisfies the same-namespace rule, adds no outward edge). - Restore InvalidHandleValues=[0] (required by RaiiFreeAttributeTests). Validated locally against MSVC 14.44: all tests pass (MetadataUtils 17/17, Win32MetadataScraper 44/44, Windows.Win32.Tests 13/13). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
Thanks for the contribution antmor! I see this is still a draft, are you still working on this? Did you want feedback on it? |
📋 API Surface Diff+38 additions / -8 deletions vs main branch (full validation run) Click to expand API diffdiff --git "a/bin\\baseline.apidump.cs" "b/bin\\current.apidump.cs"
index 955e3d9f..8bff2913 100644
--- "a/bin\\baseline.apidump.cs"
+++ "b/bin\\current.apidump.cs"
@@ -636669,7 +636669,7 @@ public static class Apis
[DllImport ("OLE32.dll", ExactSpelling = true, PreserveSig = false)]
[SupportedOSPlatform ("windows5.0")]
[Documentation ("https://learn.microsoft.com/windows/win32/api/combaseapi/nf-combaseapi-stringfromclsid")]
- public unsafe static extern HRESULT StringFromCLSID ([In][Const] Guid* rclsid, [Out] PWSTR* lplpsz);
+ public unsafe static extern HRESULT StringFromCLSID ([In][Const] Guid* rclsid, [Out][FreeWith ("CoTaskMemFree")] PWSTR* lplpsz);
[DllImport ("OLE32.dll", ExactSpelling = true, PreserveSig = false)]
[SupportedOSPlatform ("windows5.0")]
@@ -636679,7 +636679,7 @@ public static class Apis
[DllImport ("OLE32.dll", ExactSpelling = true, PreserveSig = false)]
[SupportedOSPlatform ("windows5.0")]
[Documentation ("https://learn.microsoft.com/windows/win32/api/combaseapi/nf-combaseapi-stringfromiid")]
- public unsafe static extern HRESULT StringFromIID ([In][Const] Guid* rclsid, [Out] PWSTR* lplpsz);
+ public unsafe static extern HRESULT StringFromIID ([In][Const] Guid* rclsid, [Out][FreeWith ("CoTaskMemFree")] PWSTR* lplpsz);
[DllImport ("OLE32.dll", ExactSpelling = true, PreserveSig = false)]
[SupportedOSPlatform ("windows5.0")]
@@ -889878,7 +889878,7 @@ public static class Apis
[DllImport ("SHELL32.dll", ExactSpelling = true, PreserveSig = false)]
[SupportedOSPlatform ("windows6.0.6000")]
[Documentation ("https://learn.microsoft.com/windows/win32/api/shobjidl_core/nf-shobjidl_core-shgetnamefromidlist")]
- public unsafe static extern HRESULT SHGetNameFromIDList ([In] ITEMIDLIST* pidl, [In] SIGDN sigdnName, [Out] PWSTR* ppszName);
+ public unsafe static extern HRESULT SHGetNameFromIDList ([In] ITEMIDLIST* pidl, [In] SIGDN sigdnName, [Out][FreeWith ("CoTaskMemFree")] PWSTR* ppszName);
[DllImport ("SHELL32.dll", ExactSpelling = true, PreserveSig = false)]
[SupportedOSPlatform ("windows6.1")]
@@ -890031,11 +890031,6 @@ public static class Apis
[Documentation ("https://learn.microsoft.com/windows/win32/api/shlobj_core/nf-shlobj_core-ilcombine")]
public unsafe static extern ITEMIDLIST* ILCombine ([Optional][In] ITEMIDLIST* pidl1, [Optional][In] ITEMIDLIST* pidl2);
- [DllImport ("SHELL32.dll", ExactSpelling = true, PreserveSig = false)]
- [SupportedOSPlatform ("windows5.1.2600")]
- [Documentation ("https://learn.microsoft.com/windows/win32/api/shlobj_core/nf-shlobj_core-ilfree")]
- public unsafe static extern void ILFree ([Optional][In] ITEMIDLIST* pidl);
-
[DllImport ("SHELL32.dll", ExactSpelling = true, PreserveSig = false)]
[SupportedOSPlatform ("windows5.1.2600")]
[Documentation ("https://learn.microsoft.com/windows/win32/api/shlobj_core/nf-shlobj_core-ilgetnext")]
@@ -894467,6 +894462,7 @@ public enum CommandStateChangeConstants
// ═══════════════════════════════════════════════════════════════
using System.Runtime.InteropServices;
+using Windows.Win32.Foundation.Metadata;
namespace Windows.Win32.UI.Shell.Common;
@@ -894486,6 +894482,11 @@ public static class Apis
public const uint PERCEIVEDFLAG_WMSDK = 32u;
public const uint PERCEIVEDFLAG_ZIPFOLDER = 64u;
+
+ [DllImport ("SHELL32.dll", ExactSpelling = true, PreserveSig = false)]
+ [SupportedOSPlatform ("windows5.1.2600")]
+ [Documentation ("https://learn.microsoft.com/windows/win32/api/shlobj_core/nf-shlobj_core-ilfree")]
+ public unsafe static extern void ILFree ([Optional][In] ITEMIDLIST* pidl);
}
using Windows.Win32.Foundation;
@@ -894615,6 +894616,44 @@ using Windows.Win32.Foundation.Metadata;
namespace Windows.Win32.UI.Shell.Common;
+[RAIIFree ("ILFree")]
+[AlsoUsableFor ("PIDLIST_RELATIVE")]
+[InvalidHandleValue (0L)]
+[NativeTypedef]
+public struct PIDLIST_ABSOLUTE
+{
+ public unsafe ITEMIDLIST* Value;
+}
+
+using Windows.Win32.Foundation.Metadata;
+
+namespace Windows.Win32.UI.Shell.Common;
+
+[RAIIFree ("ILFree")]
+[InvalidHandleValue (0L)]
+[NativeTypedef]
+public struct PIDLIST_RELATIVE
+{
+ public unsafe ITEMIDLIST* Value;
+}
+
+using Windows.Win32.Foundation.Metadata;
+
+namespace Windows.Win32.UI.Shell.Common;
+
+[RAIIFree ("ILFree")]
+[AlsoUsableFor ("PIDLIST_RELATIVE")]
+[InvalidHandleValue (0L)]
+[NativeTypedef]
+public struct PITEMID_CHILD
+{
+ public unsafe ITEMIDLIST* Value;
+}
+
+using Windows.Win32.Foundation.Metadata;
+
+namespace Windows.Win32.UI.Shell.Common;
+
[Documentation ("https://learn.microsoft.com/windows/win32/api/shtypes/ne-shtypes-shcolstate")]
public enum SHCOLSTATE
{
@@ -904145,7 +904184,7 @@ public interface IShellItem : IUnknown
unsafe HRESULT GetParent ([Out] IShellItem* ppsi);
[Documentation ("https://learn.microsoft.com/windows/win32/api/shobjidl_core/nf-shobjidl_core-ishellitem-getdisplayname")]
- unsafe HRESULT GetDisplayName ([In] SIGDN sigdnName, [Out] PWSTR* ppszName);
+ unsafe HRESULT GetDisplayName ([In] SIGDN sigdnName, [Out][FreeWith ("CoTaskMemFree")] PWSTR* ppszName);
[Documentation ("https://learn.microsoft.com/windows/win32/api/shobjidl_core/nf-shobjidl_core-ishellitem-getattributes")]
unsafe HRESULT GetAttributes ([In] SFGAO_FLAGS sfgaoMask, [Out] SFGAO_FLAGS* psfgaoAttribs);
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related improvements so CsWin32 consumers get RAII/free guidance for shell PIDLs and CoTaskMem-allocated strings instead of hand-rolling cleanup.
1. PIDL RAII (autoTypes.json) - BSTR-style typedef
Model
PIDLIST_RELATIVE,PIDLIST_ABSOLUTE, andPITEMID_CHILDasNativeTypedefhandles following theBSTRprecedent (a distinct typedef wrapping its real pointee,char*, with aCloseApi):{ "Name": "PIDLIST_ABSOLUTE", "ValueType": "Windows.Win32.UI.Shell.Common.ITEMIDLIST*", "CloseApi": "CoTaskMemFree", "AlsoUsableFor": "PIDLIST_RELATIVE", "NativeTypedef": true }ITEMIDLIST*pointee (novoid*), analogous to BSTR'schar*.CloseApi = CoTaskMemFree-> CsWin32 generates a SafeHandle.AlsoUsableForlinks absolute/child to relative (asHMODULE/HINSTANCEdo).BSTR/HANDLEdo).2. FreeWith(CoTaskMemFree) for CoTaskMem string out-params (emitter.settings.rsp)
The
unique_cotaskmem_stringcases -PWSTR/LPOLESTRout-params that must be freed withCoTaskMemFree, mirroring the existingSHGetKnownFolderPath::ppszPathprecedent:SHGetNameFromIDList::ppszNameStringFromCLSID::lplpszStringFromIID::lplpszIShellItem::GetDisplayName::ppszNameOpen questions / please validate in CI
ValueType(ITEMIDLIST*) has no existing precedent (all current entries use primitives/void*/IntPtr). Confirm the generator/emitter accepts a fully-qualified struct-pointer payload.CoTaskMemFreenamespace: if it resolves to multiple namespaces the same-namespace check throws; also inheritingSystem.Comrelocates the PIDL typedefs fromUI.Shell.Common- the winmd baseline diff will show this. If undesirable, alternatives are an explicitNamespaceor usingILFree(fewer partitions).Testing
Local
DoAll.ps1scrape is blocked in my environment by a VS 2026 / MSVC 14.51 vs pinned libclang 17 mismatch (__builtin_verbose_trap), unrelated to this change. Relying on CI (pr-validation) for the winmd/API diff.