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.

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

ENABLE_VIRTUAL_TERMINAL_PROCESSING is set by default in pseudo console on Windows 10 20H2. #9432

Closed
tyan0 opened this issue Mar 10, 2021 · 14 comments
Labels
Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing.

Comments

@tyan0
Copy link

tyan0 commented Mar 10, 2021

Environment

Windows 10 20H2 (Version 10.0.19042.804)

Steps to reproduce

  1. Compile the following test code.
  2. Run the test program in pseudo console (e.g. in WSL).
#include <windows.h>
#include <stdio.h>

int main()
{
	DWORD mode;
	GetConsoleMode(GetStdHandle(STD_OUTPUT_HANDLE), &mode);
	printf("%d\n", mode & ENABLE_VIRTUAL_TERMINAL_PROCESSING);
	return 0;
}

Expected behavior

At least on older Windows 10 such as 1809, the result is:

0

Actual behavior

On Windows 20H2, the result is:

4

Is this the intentional behavior?

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 10, 2021
@eryksun
Copy link

eryksun commented Mar 10, 2021

Is this the intentional behavior?

Yes. See src/host/srvinit.cpp.

@zadjii-msft
Copy link
Member

Yep, this is by design.

/dup #1965

@ghost
Copy link

ghost commented Mar 10, 2021

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@ghost ghost closed this as completed Mar 10, 2021
@ghost ghost added Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 10, 2021
@tyan0
Copy link
Author

tyan0 commented Mar 10, 2021

Thank you for your quick response. Just a question from my interest, is it possible to disable this flag by default in pseudo console? Setting HKCU\Console\VirtualTerminalLevel to 0 does not take effect.

@zadjii-msft
Copy link
Member

A better question might be: why do you want to disable VT processing? It might help to better understand your scenario.

@tyan0
Copy link
Author

tyan0 commented Mar 10, 2021

I asked just because I was interested. I don't have the scenario in which this behavior causes any problem so far.

@vefatica
Copy link

I followed the link provided by @eryksun and I'm just curious ... is the conditional expression below (from srvinit.cpp) well-formed?

bool isEnabled = false;
if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy(isEnabled) && isEnabled))

@zadjii-msft
Copy link
Member

@vefatica Sure is. That statement is basically two parts:

  1. CheckDefaultAppPolicy will get the state of the "default app policy", and stick it into the bool passed in. CheckDefaultAppPolicy will return true if it succeeded in looking that value up.
  2. Was the "default app policy" enabled?

@vefatica
Copy link

In that case I'd expect it to look like this:

if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy(&isEnabled)) && isEnabled)

@zadjii-msft
Copy link
Member

Meh, stylistic difference. Taking a param by reference means the callee doesn't need to worry about if the out param is null or not. Most of our newer code with out params (as few as those are) uses params by reference, rather than pointers.

@eryksun
Copy link

eryksun commented Mar 10, 2021

bool isEnabled = false;
if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy(isEnabled) && isEnabled))

The only argument of the SUCCEEDED macro should be the HRESULT that's returned by CheckDefaultAppPolicy, in order to get a boolean result of success or not. Negative HRESULT values indicate failure. Doing a logical && on an HRESULT and a bool is incorrect.

@vefatica
Copy link

OK, didn't know that was by reference. And the second isEnabled being inside SUCCEEDED(). It just looks really funny to me.

@zadjii-msft
Copy link
Member

oh no

@miniksa
Copy link
Member

miniksa commented Mar 10, 2021

WELP. I am fixing this as MSFT:32071839. We will replicate that back out to fix my parenthesis error in the next day or so. Good catch and thank you.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing.
Projects
None yet
Development

No branches or pull requests

5 participants