Skip to content

Conversation

jkotalik
Copy link
Contributor

The reg key flag RRF_SUBKEY_WOW6432KEY doesn't evaluate to WOW6432Node on Win8 and below. Therefore we need to insert it ourselves.

StartsWithDotnetInstallLocation is failing on CI currently.

@jkotalik jkotalik requested a review from pakrym January 11, 2019 01:34
@pakrym
Copy link
Contributor

pakrym commented Jan 11, 2019

WOW6432Node doesn't exist on 32bit windows and for 32bit apps AFAIK. You might need to make it conditional on running inside 64bit process

Remove flag parameter from TryGetString

@halter73
Copy link
Member

Another option could be to open the HKEY_LOCAL_MACHINE section using RegOpenKeyEx with the KEY_WOW64_32KEY flag which seems defined all the way back to Windows XP on both the 32-bit and 64-bit versions. Since it's a predefined registry key, there's no need to close it.

Then you could use the newly opened section/key in place of HKEY_LOCAL_MACHINE when you call TryGetString without any need to include WOW6432Node in the subsection name since the KEY_WOW64_32KEY flag should do that implicitly for you that on 64-bit systems.

Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

Needs to work on 32 bit windows

@jkotalik
Copy link
Contributor Author

🆙 📅

@jkotalik
Copy link
Contributor Author

I'm still going to play around with calling RegOpenKeyEx to get an HKEY that already contains WOW6432Node. Would be a bit cleaner than this.

@pakrym
Copy link
Contributor

pakrym commented Jan 14, 2019

I think this looks good enough.

@jkotalik
Copy link
Contributor Author

Discussed offline with @pakrym, we are going to stick with this rather than calling RegOpenKeyEx as they are functionally the same.

@jkotalik jkotalik merged commit 3f4622f into master Jan 14, 2019
@jkotalik jkotalik deleted the jkotalik/regKey branch January 14, 2019 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants