Fix #20577: Add windows symbols #2940

etcimon · 2020-02-12T00:03:37Z

dlang-bot · 2020-02-12T00:03:39Z

Thanks for your pull request and interest in making D better, @etcimon! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	20577	enhancement	Add missing symbols related to Windows UAC

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + druntime#2940"

Geod24 · 2020-02-12T00:24:35Z

https://github.com/dlang/druntime/blob/master/CONTRIBUTING.md

CyberShadow

Please maintain the style of the existing code in contributions.

CyberShadow · 2020-02-12T01:28:38Z

src/core/sys/windows/sdkddkver.d

+// set _WIN32_IE based on _WIN32_WINNT
+static if (_WIN32_WINNT <= _WIN32_WINNT_NT4)
+   enum _WIN32_IE    =  _WIN32_IE_IE50;
+else static if (_WIN32_WINNT <= _WIN32_WINNT_WIN2K)
+   enum _WIN32_IE    =  _WIN32_IE_IE501;
+else static if (_WIN32_WINNT <= _WIN32_WINNT_WINXP)
+   enum _WIN32_IE    =  _WIN32_IE_IE60;
+else static if (_WIN32_WINNT <= _WIN32_WINNT_WS03)
+   enum _WIN32_IE    =  _WIN32_IE_WS03;
+else static if (_WIN32_WINNT <= _WIN32_WINNT_VISTA)
+   enum _WIN32_IE    =  _WIN32_IE_LONGHORN;
+else static if (_WIN32_WINNT <= _WIN32_WINNT_WIN7)
+   enum _WIN32_IE    =  _WIN32_IE_WIN7;
+else static if (_WIN32_WINNT <= _WIN32_WINNT_WIN8)
+   enum _WIN32_IE    =  _WIN32_IE_WIN8;
+else
+   enum _WIN32_IE       = 0x0A00;


I think we already have this here:

druntime/src/core/sys/windows/w32api.d

Lines 48 to 84 in d7800b5

version (IE11) {

enum uint _WIN32_IE = 0xA00;

} else version (IE10) {

enum uint _WIN32_IE = 0xA00;

} else version (IE9) {

enum uint _WIN32_IE = 0x900;

} else version (IE8) {

enum uint _WIN32_IE = 0x800;

} else version (IE7) {

enum uint _WIN32_IE = 0x700;

} else version (IE602) {

enum uint _WIN32_IE = 0x603;

} else version (IE601) {

enum uint _WIN32_IE = 0x601;

} else version (IE6) {

enum uint _WIN32_IE = 0x600;

} else version (IE56) {

enum uint _WIN32_IE = 0x560;

} else version (IE55) {

enum uint _WIN32_IE = 0x550;

} else version (IE501) {

enum uint _WIN32_IE = 0x501;

} else version (IE5) {

enum uint _WIN32_IE = 0x500;

} else version (IE401) {

enum uint _WIN32_IE = 0x401;

} else version (IE4) {

enum uint _WIN32_IE = 0x400;

} else version (IE3) {

enum uint _WIN32_IE = 0x300;

} else static if (_WIN32_WINNT >= 0x500) {

enum uint _WIN32_IE = 0x600;

} else static if (_WIN32_WINNT >= 0x410) {

enum uint _WIN32_IE = 0x400;

} else {

enum uint _WIN32_IE = 0;

}

CyberShadow · 2020-02-12T01:28:55Z

src/core/sys/windows/winbase.d

@@ -2257,7 +2257,8 @@ WINBASEAPI BOOL WINAPI SetEvent(HANDLE);
    BOOL IsTextUnicode(PCVOID, int, LPINT);
    BOOL IsValidAcl(PACL);
    BOOL IsValidSecurityDescriptor(PSECURITY_DESCRIPTOR);
-    BOOL IsValidSid(PSID);
+    BOOL IsValidSid(PSID);    
+   BOOL CreateWellKnownSid(WELL_KNOWN_SID_TYPE, PSID, PSID, PDWORD);


CyberShadow · 2020-02-12T01:29:09Z

src/core/sys/windows/winnt.d

@@ -2436,6 +2437,10 @@ struct TOKEN_USER {
 }
 alias TOKEN_USER* PTOKEN_USER;

+struct TOKEN_MANDATORY_LABEL {
+   SID_AND_ATTRIBUTES Label;


Whitespace. We use an indent of 4 spaces in D source code.

CyberShadow · 2020-02-12T01:29:14Z

src/core/sys/windows/winnt.d

+enum TOKEN_ELEVATION_TYPE {
+   TokenElevationTypeDefault = 1,
+   TokenElevationTypeFull,
+   TokenElevationTypeLimited


CyberShadow · 2020-02-12T01:34:04Z

mak/WINDOWS

@@ -1202,6 +1202,9 @@ $(IMPDIR)\core\sys\windows\rpcnterr.d : src\core\sys\windows\rpcnterr.d

 $(IMPDIR)\core\sys\windows\schannel.d : src\core\sys\windows\schannel.d
 	copy $** $@
+


Please remove the tab character from this line. Trailing whitespace is not allowed by CI rules.

mak/WINDOWS

CyberShadow · 2020-02-12T03:54:23Z

src/core/sys/windows/winbase.d

@@ -2257,7 +2257,8 @@ WINBASEAPI BOOL WINAPI SetEvent(HANDLE);
    BOOL IsTextUnicode(PCVOID, int, LPINT);
    BOOL IsValidAcl(PACL);
    BOOL IsValidSecurityDescriptor(PSECURITY_DESCRIPTOR);
-    BOOL IsValidSid(PSID);
+    BOOL IsValidSid(PSID);    


Trailing whitespace, here and elsewhere (see CI log).

CyberShadow

Diff LGTM but as previously mentioned in the other PRs, please improve the commit message. It should briefly describe the changes and mention a Bugzilla issue number. See @dlang-bot 's message above and the contributing guide for details.

etcimon · 2020-02-12T11:46:57Z

I don't have a bugzilla number though, any chance I could get this merged anyways?

CyberShadow · 2020-02-12T11:49:11Z

I don't have a bugzilla number though

Just create an issue. This is one of two ways that changes get publicly documented (which all non-trivial ones should be). The other way is generally left for bigger changes, as they get a better spot in the changelog.

any chance I could get this merged anyways?

No, but I can finish the rest for you if you like.

etcimon · 2020-02-12T11:52:45Z

No, but I can finish the rest for you if you like.

To test this, I used the some D translation I made of this code: https://github.com/modzero/fix-windows-privacy/blob/master/fix-privacy-base/SelfElevate.cpp

How would I go making an issue without copying my D version of it?

CyberShadow · 2020-02-12T11:57:28Z

The issue only needs to hold a short description of the goal that this PR aims to achieve. You could add more details such as your motivation for pursuing said goal, or include D code that currently doesn't work but you wish will work, but it's not necessary. The goal is that D users who read the changelog would have something to click and read in order to understand why the change was performed and what the benefit that it brought to D was.

etcimon · 2020-02-12T12:06:07Z

The relevant issue has been added: https://issues.dlang.org/show_bug.cgi?id=20577

CyberShadow · 2020-02-12T12:07:39Z

Great. Now you need to edit the commit message (not PR title/description) to reference the issue number. E.g. you could use:

Fix issue 20577 - Add missing symbols related to Windows UAC

etcimon · 2020-02-12T12:07:41Z

Should I use the issue number in the commit description as well? Or just the title of the PR is fine?

Edit: nvm

CyberShadow

Nit, commit messages should be wrapped at 60-80 characters. (https://chris.beams.io/posts/git-commit/ and elsewhere)

Add extern(Windows) symbols and types related to Windows UAC elevation and improve related types and enums.

etcimon requested a review from CyberShadow as a code owner February 12, 2020 00:03

etcimon force-pushed the add_symbols branch from abdb3aa to 38ea13f Compare February 12, 2020 00:08

etcimon requested review from andralex and wilzbach as code owners February 12, 2020 00:08

etcimon force-pushed the add_symbols branch from 38ea13f to 923236b Compare February 12, 2020 00:17

CyberShadow reviewed Feb 12, 2020

View reviewed changes

etcimon force-pushed the add_symbols branch 4 times, most recently from 7c72c4d to e40186e Compare February 12, 2020 02:47

CyberShadow reviewed Feb 12, 2020

View reviewed changes

mak/WINDOWS Show resolved Hide resolved

CyberShadow reviewed Feb 12, 2020

View reviewed changes

etcimon force-pushed the add_symbols branch from e40186e to 0a5d4c1 Compare February 12, 2020 05:52

CyberShadow reviewed Feb 12, 2020

View reviewed changes

etcimon force-pushed the add_symbols branch from 0a5d4c1 to c77b6ac Compare February 12, 2020 11:43

etcimon changed the title ~~Add windows symbols~~ Fix #20577: Add windows symbols Feb 12, 2020

etcimon force-pushed the add_symbols branch from c77b6ac to afcef4f Compare February 12, 2020 12:08

dlang-bot added the Enhancement New functionality label Feb 12, 2020

CyberShadow approved these changes Feb 12, 2020

View reviewed changes

Fix issue 20577 - Add missing symbols related to Windows UAC

c8ca9b3

Add extern(Windows) symbols and types related to Windows UAC elevation and improve related types and enums.

etcimon force-pushed the add_symbols branch from afcef4f to c8ca9b3 Compare February 12, 2020 12:11

thewilsonator added the auto-merge label Feb 13, 2020

dlang-bot merged commit a581dc6 into dlang:master Feb 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #20577: Add windows symbols #2940

Fix #20577: Add windows symbols #2940

etcimon commented Feb 12, 2020

dlang-bot commented Feb 12, 2020 •

edited

Loading

Geod24 commented Feb 12, 2020

CyberShadow left a comment

CyberShadow Feb 12, 2020

CyberShadow Feb 12, 2020

CyberShadow Feb 12, 2020 •

edited

Loading

CyberShadow Feb 12, 2020

CyberShadow Feb 12, 2020 •

edited

Loading

CyberShadow Feb 12, 2020

CyberShadow left a comment •

edited

Loading

etcimon commented Feb 12, 2020

CyberShadow commented Feb 12, 2020

etcimon commented Feb 12, 2020

CyberShadow commented Feb 12, 2020

etcimon commented Feb 12, 2020

CyberShadow commented Feb 12, 2020

etcimon commented Feb 12, 2020 •

edited

Loading

CyberShadow left a comment •

edited

Loading

	version (IE11) {
	enum uint _WIN32_IE = 0xA00;
	} else version (IE10) {
	enum uint _WIN32_IE = 0xA00;
	} else version (IE9) {
	enum uint _WIN32_IE = 0x900;
	} else version (IE8) {
	enum uint _WIN32_IE = 0x800;
	} else version (IE7) {
	enum uint _WIN32_IE = 0x700;
	} else version (IE602) {
	enum uint _WIN32_IE = 0x603;
	} else version (IE601) {
	enum uint _WIN32_IE = 0x601;
	} else version (IE6) {
	enum uint _WIN32_IE = 0x600;
	} else version (IE56) {
	enum uint _WIN32_IE = 0x560;
	} else version (IE55) {
	enum uint _WIN32_IE = 0x550;
	} else version (IE501) {
	enum uint _WIN32_IE = 0x501;
	} else version (IE5) {
	enum uint _WIN32_IE = 0x500;
	} else version (IE401) {
	enum uint _WIN32_IE = 0x401;
	} else version (IE4) {
	enum uint _WIN32_IE = 0x400;
	} else version (IE3) {
	enum uint _WIN32_IE = 0x300;
	} else static if (_WIN32_WINNT >= 0x500) {
	enum uint _WIN32_IE = 0x600;
	} else static if (_WIN32_WINNT >= 0x410) {
	enum uint _WIN32_IE = 0x400;
	} else {
	enum uint _WIN32_IE = 0;
	}

		@@ -1202,6 +1202,9 @@ $(IMPDIR)\core\sys\windows\rpcnterr.d : src\core\sys\windows\rpcnterr.d

		$(IMPDIR)\core\sys\windows\schannel.d : src\core\sys\windows\schannel.d
		copy $** $@

Fix #20577: Add windows symbols #2940

Fix #20577: Add windows symbols #2940

Conversation

etcimon commented Feb 12, 2020

dlang-bot commented Feb 12, 2020 • edited Loading

Bugzilla references

Testing this PR locally

Geod24 commented Feb 12, 2020

CyberShadow left a comment

Choose a reason for hiding this comment

CyberShadow Feb 12, 2020

Choose a reason for hiding this comment

CyberShadow Feb 12, 2020

Choose a reason for hiding this comment

CyberShadow Feb 12, 2020 • edited Loading

Choose a reason for hiding this comment

CyberShadow Feb 12, 2020

Choose a reason for hiding this comment

CyberShadow Feb 12, 2020 • edited Loading

Choose a reason for hiding this comment

CyberShadow Feb 12, 2020

Choose a reason for hiding this comment

CyberShadow left a comment • edited Loading

Choose a reason for hiding this comment

etcimon commented Feb 12, 2020

CyberShadow commented Feb 12, 2020

etcimon commented Feb 12, 2020

CyberShadow commented Feb 12, 2020

etcimon commented Feb 12, 2020

CyberShadow commented Feb 12, 2020

etcimon commented Feb 12, 2020 • edited Loading

CyberShadow left a comment • edited Loading

Choose a reason for hiding this comment

dlang-bot commented Feb 12, 2020 •

edited

Loading

CyberShadow Feb 12, 2020 •

edited

Loading

CyberShadow Feb 12, 2020 •

edited

Loading

CyberShadow left a comment •

edited

Loading

etcimon commented Feb 12, 2020 •

edited

Loading

CyberShadow left a comment •

edited

Loading