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

Add internal registry implementation for win32 #11137

Conversation

straight-shoota
Copy link
Member

This is a minimal internal implementation to provide registry access on win32. It is necessary for looking up time zone names and file extensions for the MIME database. Both these use cases are already included.

It is an alternative to the more feature complete, public implementation in #6698 and implements only the minimal required parts and exposes no platform-specific APIs.

@straight-shoota straight-shoota added kind:feature platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:time topic:stdlib:files labels Aug 25, 2021
@straight-shoota
Copy link
Member Author

I had to do some incremental debugging in CI to get the time spec to work (it seems the CI runners have UTC as their default timezone which requires a complete time zone setup).

This should now be ready for reviews.

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

I left some comments to help understand a bit some edge cases, but in the overall it looks good.

src/crystal/system/win32/windows_registry.cr Show resolved Hide resolved
src/crystal/system/win32/windows_registry.cr Show resolved Hide resolved
elsif small_buf && length > 0
next length
else
raise "RegQueryValueExW retry buffer"
Copy link
Member

Choose a reason for hiding this comment

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

In particular, when is this error expected? Is it an unexpected failure?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's never expected. The block runs maximal two times, the first time it either succeeds directly or returns the buffer size to be allocated. On the second run, the buffer should be approriately sized. If it's not that's points to an error in the lib function or a race condition.

src/crystal/system/win32/windows_registry.cr Outdated Show resolved Hide resolved
@beta-ziliani beta-ziliani added this to the 1.2.0 milestone Sep 8, 2021
@straight-shoota straight-shoota merged commit 48aaf58 into crystal-lang:master Sep 10, 2021
@straight-shoota straight-shoota deleted the feature/win32-registry-internal branch September 10, 2021 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:files topic:stdlib:time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants