Skip to content
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

Jump to bottom

Cleanup PowerStatus interop #2041

Merged
merged 2 commits into from
Oct 10, 2019
Merged

Cleanup PowerStatus interop #2041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85df04e
Cleanup PowerStatus interop
hughbe Oct 6, 2019
cd861f3
Use BOOLEAN instead of bool
hughbe Oct 8, 2019
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
1 change: 0 additions & 1 deletion src/Common/src/ExternDll.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ internal static class ExternDll
public const string Oleaut32 = "oleaut32.dll";
public const string Olepro32 = "olepro32.dll";
public const string PerfCounter = "perfcounter.dll";
public const string Powrprof = "Powrprof.dll";
public const string Psapi = "psapi.dll";
public const string Shell32 = "shell32.dll";
public const string User32 = "user32.dll";
Expand Down
30 changes: 30 additions & 0 deletions src/Common/src/Interop/Interop.BOOLEAN.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using static Interop;

internal partial class Interop
{
/// <summary>
/// Blittable version of Windows BOOLEAN type. It is convenient in situations where
/// manual marshalling is required, or to avoid overhead of regular BOOLEAN marshalling.
/// </summary>
/// <remarks>
/// Some Windows APIs return arbitrary integer values although the return type is defined
/// as BOOL. It is best to never compare BOOLEAN to TRUE. Always use bResult != BOOLEAN.FALSE
/// or bResult == BOOLEAN.FALSE .
/// </remarks>
internal enum BOOLEAN : byte
{
FALSE = 0,
TRUE = 1,
}
}

internal static class BooleanExtensions
{
public static bool IsTrue(this BOOLEAN b) => b != BOOLEAN.FALSE;
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be more prudent to override the operators !, !=, and == for comparisons to both BOOLEAN and bools

Also, maybe I am mistaken, but isn't there already a BOOL which is identical to this? Is there no way to make both BOOLEAN and BOOL extend some common type which extends byte and then add these helper functions to that underlying type? Just trying to avoid duplicating code

Copy link
Member

Choose a reason for hiding this comment

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

You can't unless you make it a struct instead of an enum. It can be done per my example, it just isn't something the Core architects liked as it isn't technically "correct" and enums can potentially get better optimizations. It does work, however, and makes usage pretty easy, imho.

Copy link
Contributor

@weltkante weltkante Oct 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

isn't there already a BOOL which is identical to this?

@zsd4yr no, BOOL and BOOLEAN are different sizes in the headers (which was the reason BOOLEAN is added here)

make it a struct instead of an enum [...] It does work, however

@JeremyKuhne only due to bug-for-bug compatibility with Desktop Framework, structs can trigger different behavior in calling conventions than primitives, passing them as a stack pointer instead of by-value in a register. Initially .NET Core caused crashes in WPF when using HRESULT struct wrappers as return values, they restored the bug from Desktop to keep this particular scenario compatible but I'm not entirely sure it will work in other situations, considering that the calling conventions can make differences between wrapper structs and wrapped values, so they are a bit of a minefield.

relevant discussions: [1] [2] [3]

Technical TLDR: the native calling conventions for returning structs in

  • SomeStruct(__stdcall SomeInterface::*SomeMethod)(); and
  • SomeStruct(__stdcall *SomeMethod)(SomeInterface*);

(i.e. passing this explicitely or implicitely) are not identical and .NET cannot differentiate between them, historically it has been mixed up due to a bug, and today (for compatibility) it has to guess looking at the struct which one is more likely what you meant, if it happens to chose the wrong case for your usage of the struct you crash or get garbage results.

Copy link
Contributor

Choose a reason for hiding this comment

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

See also please #1978 (comment)

public static bool IsFalse(this BOOLEAN b) => b == BOOLEAN.FALSE;
public static BOOLEAN ToBOOLEAN(this bool b) => b ? BOOLEAN.TRUE : BOOLEAN.FALSE;
}
1 change: 1 addition & 0 deletions src/Common/src/Interop/Interop.Libraries.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public static partial class Libraries
public const string NtDll = "ntdll.dll";
public const string Ole32 = "ole32.dll";
public const string Oleaut32 = "oleaut32.dll";
public const string Powrprof = "Powrprof.dll";
public const string RichEdit41 = "MsftEdit.DLL";
public const string Shell32 = "shell32.dll";
public const string Shlwapi = "shlwapi.dll";
Expand Down
14 changes: 14 additions & 0 deletions src/Common/src/Interop/Kernel32/Interop.GetSystemPowerStatus.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Runtime.InteropServices;

internal partial class Interop
{
internal partial class Kernel32
{
[DllImport(Libraries.Kernel32, ExactSpelling = true)]
public static extern BOOL GetSystemPowerStatus(ref SYSTEM_POWER_STATUS lpSystemPowerStatus);
}
}
19 changes: 19 additions & 0 deletions src/Common/src/Interop/Kernel32/Interop.SYSTEM_POWER_STATUS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

internal partial class Interop
{
internal partial class Kernel32
{
public struct SYSTEM_POWER_STATUS
{
public byte ACLineStatus;
public byte BatteryFlag;
public byte BatteryLifePercent;
public byte Reserved1;
public int BatteryLifeTime;
public int BatteryFullLifeTime;
}
}
}
14 changes: 14 additions & 0 deletions src/Common/src/Interop/Powrprof/Interop.SetSuspendState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Runtime.InteropServices;

internal partial class Interop
{
internal partial class Powrprof
{
[DllImport(Libraries.Powrprof, ExactSpelling = true, SetLastError = true)]
public static extern BOOLEAN SetSuspendState(BOOLEAN bHibernate, BOOLEAN bForce, BOOLEAN bWakeupEventsDisabled);
RussKie marked this conversation as resolved.
Show resolved Hide resolved
}
}
11 changes: 0 additions & 11 deletions src/Common/src/NativeMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3336,17 +3336,6 @@ internal static string GetLocalPath(string fileName)
return uri.LocalPath + uri.Fragment;
}

[StructLayout(LayoutKind.Sequential)]
public struct SYSTEM_POWER_STATUS
{
public byte ACLineStatus;
public byte BatteryFlag;
public byte BatteryLifePercent;
public byte Reserved1;
public int BatteryLifeTime;
public int BatteryFullLifeTime;
}

public enum PROCESS_DPI_AWARENESS
{
PROCESS_DPI_UNINITIALIZED = -1,
Expand Down
8 changes: 0 additions & 8 deletions src/Common/src/UnsafeNativeMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,14 +622,6 @@ public static IntPtr SetWindowLong(HandleRef hWnd, int nIndex, NativeMethods.Wnd
[DllImport(ExternDll.User32, ExactSpelling = true, SetLastError = true)]
public static extern uint GetDpiForWindow(HandleRef hWnd);

// For system power status
//
[DllImport(ExternDll.Kernel32, ExactSpelling = true, CharSet = CharSet.Auto)]
public static extern bool GetSystemPowerStatus([In, Out] ref NativeMethods.SYSTEM_POWER_STATUS systemPowerStatus);

[DllImport(ExternDll.Powrprof, ExactSpelling = true, CharSet = CharSet.Auto)]
public static extern bool SetSuspendState(bool hiberate, bool forceCritical, bool disableWakeEvent);

//for RegionData
[DllImport(ExternDll.Gdi32, SetLastError = true, ExactSpelling = true, CharSet = System.Runtime.InteropServices.CharSet.Auto)]
public static extern int GetRegionData(HandleRef hRgn, int size, IntPtr lpRgnData);
Expand Down
1 change: 0 additions & 1 deletion src/System.Windows.Forms.Design.Editors/src/Misc/ExternDll.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ internal static class ExternDll
public const string Oleaut32 = "oleaut32.dll";
public const string Olepro32 = "olepro32.dll";
public const string PerfCounter = "perfcounter.dll";
public const string Powrprof = "Powrprof.dll";
public const string Psapi = "psapi.dll";
public const string Shell32 = "shell32.dll";
public const string User32 = "user32.dll";
Expand Down
2 changes: 1 addition & 1 deletion src/System.Windows.Forms/src/System/Windows/Forms/Application.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ public static void SetCompatibleTextRenderingDefault(bool defaultValue)
/// Returns true if the call succeeded, else false.
/// </summary>
public static bool SetSuspendState(PowerState state, bool force, bool disableWakeEvent)
=> UnsafeNativeMethods.SetSuspendState(state == PowerState.Hibernate, force, disableWakeEvent);
=> Powrprof.SetSuspendState((state == PowerState.Hibernate).ToBOOLEAN(), force.ToBOOLEAN(), disableWakeEvent.ToBOOLEAN()).IsTrue();

/// <summary>
/// Overload version of SetUnhandledExceptionMode that sets the UnhandledExceptionMode
Expand Down
9 changes: 4 additions & 5 deletions src/System.Windows.Forms/src/System/Windows/Forms/PowerStatus.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using static Interop;

namespace System.Windows.Forms
{
public class PowerStatus
{
private NativeMethods.SYSTEM_POWER_STATUS _systemPowerStatus;
private Kernel32.SYSTEM_POWER_STATUS _systemPowerStatus;

internal PowerStatus()
{
Expand Down Expand Up @@ -58,9 +60,6 @@ public int BatteryLifeRemaining
}
}

private void UpdateSystemPowerStatus()
{
UnsafeNativeMethods.GetSystemPowerStatus(ref _systemPowerStatus);
}
private void UpdateSystemPowerStatus() => Kernel32.GetSystemPowerStatus(ref _systemPowerStatus);
}
}