Skip to content

chore: migrate P/Invoke declarations to [LibraryImport] (MODERN-002) #454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laurentiu021 merged 1 commit into from
May 20, 2026
Merged

chore: migrate P/Invoke declarations to [LibraryImport] (MODERN-002) #454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions SysManager/SysManager/App.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,17 @@ public partial class App : Application
/// <summary>The shared tray icon service instance.</summary>
public TrayIconService? TrayService => _trayService;

[DllImport("user32.dll")]
private static extern bool SetForegroundWindow(IntPtr hWnd);
[LibraryImport("user32.dll")]
[return: MarshalAs(UnmanagedType.Bool)]
private static partial bool SetForegroundWindow(IntPtr hWnd);

[DllImport("user32.dll")]
private static extern bool ShowWindow(IntPtr hWnd, int nCmdShow);
[LibraryImport("user32.dll")]
[return: MarshalAs(UnmanagedType.Bool)]
private static partial bool ShowWindow(IntPtr hWnd, int nCmdShow);

[DllImport("user32.dll")]
[LibraryImport("user32.dll")]
[return: MarshalAs(UnmanagedType.Bool)]
private static extern bool IsIconic(IntPtr hWnd);
private static partial bool IsIconic(IntPtr hWnd);

private const int SW_RESTORE = 9;

Expand Down
8 changes: 4 additions & 4 deletions SysManager/SysManager/Helpers/KnownFolders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace SysManager.Helpers;
/// Handles cases where the user has moved default folders (Downloads,
/// Documents, Desktop, etc.) to a non-standard location.
/// </summary>
internal static class KnownFolders
internal static partial class KnownFolders
{
// Known Folder GUIDs — https://learn.microsoft.com/en-us/windows/win32/shell/knownfolderid
private static readonly Guid Downloads = new("374DE290-123F-4565-9164-39C4925E467B");
Expand All @@ -22,9 +22,9 @@ internal static class KnownFolders
private static readonly Guid Music = new("4BD8D571-6D19-48D3-BE97-422220080E43");
private static readonly Guid Videos = new("18989B1D-99B5-455B-841C-AB7C74E4DDFC");

[DllImport("shell32.dll", CharSet = CharSet.Unicode, ExactSpelling = true, PreserveSig = false)]
private static extern void SHGetKnownFolderPath(
[MarshalAs(UnmanagedType.LPStruct)] Guid rfid,
[LibraryImport("shell32.dll", StringMarshalling = StringMarshalling.Utf16)]
private static partial void SHGetKnownFolderPath(
in Guid rfid,
uint dwFlags,
nint hToken,
out string pszPath);
Comment on lines +25 to 30

@coderabbitai coderabbitai Bot May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does C# [LibraryImport] support automatic HRESULT-to-exception translation or a PreserveSig=false equivalent for Win32 APIs like SHGetKnownFolderPath? What is the recommended LibraryImport signature for SHGetKnownFolderPath when the native function returns HRESULT and allocates a PWSTR that must be freed by the caller?

💡 Result:

Yes. When you use source-generated P/Invoke via LibraryImport (LibraryImportAttribute),.NET performs the same kind of HRESULT-to-exception translation that DllImportAttribute does when PreserveSig=false. In that mode, failing HRESULTs are automatically converted into exceptions, and successful S_OK (0) is treated as “no exception”; the managed signature becomes void (or the type of the last [out, retval] parameter) rather than returning the HRESULT directly. [1][2][3] For SHGetKnownFolderPath, the native signature is: HRESULT SHGetKnownFolderPath(..., PWSTR ppszPath); The output string is allocated by the callee and must always be freed by the caller with CoTaskMemFree once you’re done (even on failure). [4] Recommended LibraryImport signature (HRESULT-returning native, PWSTR allocated by callee freed by caller): [LibraryImport("shell32.dll", StringMarshalling = StringMarshalling.Utf16)] internal static partial int SHGetKnownFolderPath( ref readonly Guid rfid, uint dwFlags, IntPtr hToken, out IntPtr ppszPath); Then, call it and translate HRESULT yourself (because you will typically keep the HRESULT return type as int). Example pattern: - declare IntPtr p; - int hr = SHGetKnownFolderPath(ref knownFolderId, flags, IntPtr.Zero, out p); - if (p!= IntPtr.Zero) { string path = Marshal.PtrToStringUni(p)!; Marshal.FreeCoTaskMem(p); } If you want automatic exception translation, you need the LibraryImport signature to be “PreserveSig=false equivalent” (i.e., have.NET treat the HRESULT as not directly returned). Microsoft documents that setting PreserveSig=false causes the managed signature to become void or the type of the last [out, retval] parameter, and failing HRESULTs throw automatically. [1][3] However, for SHGetKnownFolderPath specifically, because the last parameter is a pointer-out (PWSTR), there is no built-in way described in the cited docs to both (a) keep a success/failure automatic exception translation and (b) return the allocated pointer safely without also relying on correct source-generator handling of the parameter as the “retval”/translated return. The most robust, recommended approach grounded in the official allocation/freeing contract is therefore: keep an int HRESULT return and explicitly free the returned PWSTR with CoTaskMemFree/Marshal.FreeCoTaskMem. [4]

Citations:

🏁 Script executed:

# Locate and examine the KnownFolders.cs file
fd -t f "KnownFolders.cs" --exec wc -l {} \;

Repository: laurentiu021/SystemManager

Length of output: 121

🏁 Script executed:

# Read the file to verify the code and fallback logic
fd -t f "KnownFolders.cs" --exec cat -n {} \;

Repository: laurentiu021/SystemManager

Length of output: 3948

Preserve the HRESULT from SHGetKnownFolderPath for explicit error handling.

The current declaration with StringMarshalling = StringMarshalling.Utf16 and void return type relies on automatic HRESULT-to-exception translation. However, this combination is not reliably specified for LibraryImport with out-pointer parameters like PWSTR*. The native function allocates memory that must be freed with CoTaskMemFree; using out string obscures this contract. Return the int HRESULT, explicitly check it with Marshal.ThrowExceptionForHR(), and manually free the allocated pointer to ensure correct fallback behavior and memory safety.

Suggested fix 
-    [LibraryImport("shell32.dll", StringMarshalling = StringMarshalling.Utf16)]
-    private static partial void SHGetKnownFolderPath(
+    [LibraryImport("shell32.dll")]
+    private static partial int SHGetKnownFolderPath(
         in Guid rfid,
         uint dwFlags,
         nint hToken,
-        out string pszPath);
+        out nint pszPath);
     private static string GetPath(Guid folderId)
     {
         try
         {
-            SHGetKnownFolderPath(folderId, 0, nint.Zero, out var path);
-            return path;
+            var hr = SHGetKnownFolderPath(folderId, 0, nint.Zero, out var pathPtr);
+            if (hr < 0)
+                Marshal.ThrowExceptionForHR(hr);
+
+            try
+            {
+                return Marshal.PtrToStringUni(pathPtr) ?? string.Empty;
+            }
+            finally
+            {
+                Marshal.FreeCoTaskMem(pathPtr);
+            }
         }
         catch (COMException)
         {
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SysManager/SysManager/Helpers/KnownFolders.cs` around lines 25 - 30, Change
the SHGetKnownFolderPath P/Invoke to return the HRESULT (int) and use an out
IntPtr (pointer) instead of out string so you can check the result and manage
the native memory yourself: update the LibraryImport signature for
SHGetKnownFolderPath to return int and out IntPtr pszPath (remove
StringMarshalling for that out param), then in the caller call
SHGetKnownFolderPath, check the returned HRESULT and call
Marshal.ThrowExceptionForHR(hresult) on failure, convert the returned IntPtr to
a managed string with Marshal.PtrToStringUni on success, and free the native
buffer with Marshal.FreeCoTaskMem (CoTaskMemFree) after conversion to avoid
leaks and ensure correct fallback behavior.

Expand Down
4 changes: 2 additions & 2 deletions SysManager/SysManager/MainWindow.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ private IntPtr WndProc(IntPtr hwnd, int msg, IntPtr wParam, IntPtr lParam, ref b
return IntPtr.Zero;
}

[System.Runtime.InteropServices.DllImport("user32.dll")]
private static extern IntPtr DefWindowProc(IntPtr hWnd, int msg, IntPtr wParam, IntPtr lParam);
[System.Runtime.InteropServices.LibraryImport("user32.dll")]
private static partial IntPtr DefWindowProc(IntPtr hWnd, int msg, IntPtr wParam, IntPtr lParam);

/// <summary>Click on a single-item group (Dashboard, Network).</summary>
private void SingleGroup_Click(object sender, MouseButtonEventArgs e)
Expand Down
14 changes: 7 additions & 7 deletions SysManager/SysManager/Services/IconExtractorService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace SysManager.Services;
/// icons (Windows logo for system processes, gear for services, generic app icon).
/// Results are cached per-path to avoid repeated extraction.
/// </summary>
public sealed class IconExtractorService
public sealed partial class IconExtractorService
{
private static readonly ConcurrentDictionary<string, ImageSource?> _cache = new(StringComparer.OrdinalIgnoreCase);
private static readonly object _evictionLock = new();
Expand Down Expand Up @@ -203,7 +203,7 @@ public sealed class IconExtractorService
var shell32 = Path.Join(
Environment.GetFolderPath(Environment.SpecialFolder.System), "shell32.dll");

var hIcon = ExtractIconW(IntPtr.Zero, shell32, index);
var hIcon = ExtractIcon(IntPtr.Zero, shell32, index);
if (hIcon == IntPtr.Zero || hIcon == (IntPtr)1)
return null;

Expand Down Expand Up @@ -470,15 +470,15 @@ private struct SHFILEINFO
public string szTypeName;
}

[DllImport("shell32.dll", CharSet = CharSet.Auto)]
[DllImport("shell32.dll", CharSet = CharSet.Unicode)]
private static extern IntPtr SHGetFileInfo(
string pszPath, uint dwFileAttributes,
ref SHFILEINFO psfi, uint cbSizeFileInfo, uint uFlags);

[DllImport("shell32.dll", CharSet = CharSet.Auto)]
private static extern IntPtr ExtractIconW(IntPtr hInst, string lpszExeFileName, int nIconIndex);
[LibraryImport("shell32.dll", StringMarshalling = StringMarshalling.Utf16, EntryPoint = "ExtractIconW")]
private static partial IntPtr ExtractIcon(IntPtr hInst, string lpszExeFileName, int nIconIndex);

[DllImport("user32.dll", SetLastError = true)]
[LibraryImport("user32.dll", SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
private static extern bool DestroyIcon(IntPtr hIcon);
private static partial bool DestroyIcon(IntPtr hIcon);
}
26 changes: 13 additions & 13 deletions SysManager/SysManager/Services/PerformanceService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace SysManager.Services;
/// • NVIDIA GPU subkey is auto-detected (not hardcoded to 0000).
/// • Visual effects use SystemParametersInfo (instant), not registry-only.
/// </summary>
public sealed class PerformanceService : IDisposable
public sealed partial class PerformanceService : IDisposable
{
private readonly PowerShellRunner _ps;
private readonly SemaphoreSlim _psGate = new(1, 1);
Expand All @@ -46,15 +46,15 @@ public sealed class PerformanceService : IDisposable
private const uint SPI_SETUIEFFECTS = 0x103F;
private const uint SPIF_SENDCHANGE = 0x02;

[DllImport("user32.dll", SetLastError = true)]
[LibraryImport("user32.dll", EntryPoint = "SystemParametersInfoW", SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
private static extern bool SystemParametersInfo(
uint uiAction, uint uiParam, ref bool pvParam, uint fWinIni);
private static partial bool SystemParametersInfoGet(
uint uiAction, uint uiParam, ref int pvParam, uint fWinIni);

[DllImport("user32.dll", SetLastError = true)]
[LibraryImport("user32.dll", EntryPoint = "SystemParametersInfoW", SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
private static extern bool SystemParametersInfo(
uint uiAction, uint uiParam, bool pvParam, uint fWinIni);
private static partial bool SystemParametersInfoSet(
uint uiAction, uint uiParam, int pvParam, uint fWinIni);

public PerformanceService(PowerShellRunner ps) => _ps = ps;

Expand Down Expand Up @@ -266,9 +266,9 @@ public async Task<string> EnsureUltimatePerformancePlanAsync(CancellationToken c
/// <summary>Read whether UI effects are currently enabled.</summary>
internal static bool GetUiEffectsEnabled()
{
bool enabled = true;
SystemParametersInfo(SPI_GETUIEFFECTS, 0, ref enabled, 0);
return enabled;
int enabled = 1;
SystemParametersInfoGet(SPI_GETUIEFFECTS, 0, ref enabled, 0);
return enabled != 0;
Comment on lines +269 to +271

@coderabbitai coderabbitai Bot May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

For .NET [LibraryImport] with SetLastError = true, should callers use Marshal.GetLastPInvokeError() and explicitly check the BOOL returned by SystemParametersInfoW for SPI_GETUIEFFECTS and SPI_SETUIEFFECTS?

💡 Result:

When using.NET LibraryImport with SetLastError = true, you should primarily rely on the boolean return value of SystemParametersInfoW to determine if the call succeeded [1][2]. According to the Windows API documentation, SystemParametersInfo returns a nonzero value (TRUE) if the function succeeds and zero (FALSE) if it fails [1][2]. You should check this return value first. If the function returns FALSE, you can then use Marshal.GetLastPInvokeError to retrieve the extended error information [3][4]. This is the correct, modern.NET approach for accessing the last Win32 error code after a P/Invoke call configured with SetLastError = true [3][4]. It is important to note that GetLastError (and by extension, Marshal.GetLastPInvokeError) should only be consulted when the function's return value indicates a failure [5][6]. If the function succeeds, the value returned by GetLastPInvokeError is unreliable and may contain stale data from previous operations [5][6]. Therefore, explicitly checking the boolean return value is the standard and required practice for robust error handling with this API [5][6].

Citations:

🏁 Script executed:

# Find and examine the PerformanceService.cs file
find . -name "PerformanceService.cs" -type f

Repository: laurentiu021/SystemManager

Length of output: 125

🏁 Script executed:

# Once we locate it, examine the relevant lines and method signatures
cat -n SysManager/SysManager/Services/PerformanceService.cs | sed -n '250,300p'

Repository: laurentiu021/SystemManager

Length of output: 2423

🏁 Script executed:

# Check the full file size to understand context
wc -l SysManager/SysManager/Services/PerformanceService.cs

Repository: laurentiu021/SystemManager

Length of output: 127

🏁 Script executed:

# Search for the SystemParametersInfoGet and SystemParametersInfoSet declarations
rg -n "SystemParametersInfo(Get|Set)" SysManager/SysManager/Services/PerformanceService.cs

Repository: laurentiu021/SystemManager

Length of output: 356

🏁 Script executed:

# Look for LibraryImport and SetLastError declarations
rg -n -B 2 "SystemParametersInfo" SysManager/SysManager/Services/PerformanceService.cs | head -50

Repository: laurentiu021/SystemManager

Length of output: 1283

Check the SystemParametersInfoW return value before trusting the result.

Both call sites discard the BOOL return. If the GET call fails, Line 271 reports true from the seeded value; if the SET call fails, Line 283 still logs success even though nothing changed.

Suggested fix 
     internal static bool GetUiEffectsEnabled()
     {
         int enabled = 1;
-        SystemParametersInfoGet(SPI_GETUIEFFECTS, 0, ref enabled, 0);
+        if (!SystemParametersInfoGet(SPI_GETUIEFFECTS, 0, ref enabled, 0))
+            throw new System.ComponentModel.Win32Exception(Marshal.GetLastPInvokeError());
         return enabled != 0;
     }
@@
     public static void SetUiEffects(bool enabled)
     {
-        SystemParametersInfoSet(SPI_SETUIEFFECTS, 0, enabled ? 1 : 0, SPIF_SENDCHANGE);
+        if (!SystemParametersInfoSet(SPI_SETUIEFFECTS, 0, enabled ? 1 : 0, SPIF_SENDCHANGE))
+            throw new System.ComponentModel.Win32Exception(Marshal.GetLastPInvokeError());
         Log.Information("System: Visual effects {Action}", enabled ? "enabled" : "reduced");
     }

Also applies to: 282-283

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SysManager/SysManager/Services/PerformanceService.cs` around lines 269 - 271,
The code is currently ignoring the BOOL return from the native calls; update the
usages of SystemParametersInfoGet and the corresponding setter (e.g.,
SystemParametersInfoSet/SystemParametersInfo with SPI_GETUIEFFECTS and
SPI_SETUIEFFECTS) to check their return value, and if the call fails, do not
assume the seeded/default value or log success—instead log or propagate an error
that includes Marshal.GetLastWin32Error() and return a failure/false result (or
throw) so callers know the operation did not succeed; only treat the operation
as successful when the native BOOL return is true.

}

/// <summary>
Expand All @@ -279,7 +279,7 @@ internal static bool GetUiEffectsEnabled()
/// </summary>
public static void SetUiEffects(bool enabled)
{
SystemParametersInfo(SPI_SETUIEFFECTS, 0, enabled, SPIF_SENDCHANGE);
SystemParametersInfoSet(SPI_SETUIEFFECTS, 0, enabled ? 1 : 0, SPIF_SENDCHANGE);
Log.Information("System: Visual effects {Action}", enabled ? "enabled" : "reduced");
}

Expand Down Expand Up @@ -531,9 +531,9 @@ public async Task<bool> CreateRestorePointAsync(string description, Cancellation
// RAM WORKING SET TRIM — via EmptyWorkingSet (instant)
// ═══════════════════════════════════════════════════════════════

[DllImport("psapi.dll", SetLastError = true)]
[LibraryImport("psapi.dll", SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
private static extern bool EmptyWorkingSet(IntPtr hProcess);
private static partial bool EmptyWorkingSet(IntPtr hProcess);

/// <summary>
/// Trim the working set of all accessible processes, freeing physical
Expand Down
2 changes: 1 addition & 1 deletion SysManager/SysManager/Services/ShortcutCleanerService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace SysManager.Services;
/// Scans common locations for .lnk shortcuts whose targets no longer exist.
/// Supports deletion to Recycle Bin or permanent delete.
/// </summary>
public sealed class ShortcutCleanerService
public sealed partial class ShortcutCleanerService
{
/// <summary>
/// Scans all common shortcut locations and returns broken shortcuts.
Expand Down
1 change: 1 addition & 0 deletions SysManager/SysManager/SysManager.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<AssemblyName>SysManager</AssemblyName>
<RootNamespace>SysManager</RootNamespace>
<LangVersion>latest</LangVersion>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Version>0.48.21</Version>
<FileVersion>0.48.21.0</FileVersion>
<AssemblyVersion>0.48.21.0</AssemblyVersion>
Expand Down
Loading