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

[pzstd] Fixes for Windows build #3380

Merged
merged 3 commits into from
Dec 19, 2022
Merged

[pzstd] Fixes for Windows build #3380

merged 3 commits into from
Dec 19, 2022

Conversation

terrelln
Copy link
Contributor

@terrelln terrelln commented Dec 19, 2022

  • Add Portability.h to fix min/max issues
  • Fix conversion warnings
  • Assert that windowLog <= 23, which is currently always the case.
    This could be loosened, but we aren't looking to add new functionality.

Fixes on top of PR #3375 by @eli-schwartz, which added Windows CI for contrib & programs.

@terrelln
Copy link
Contributor Author

Looks like CI is passing now.

// Required for windows, which defines min/max, but we want the std:: version.

#ifdef min
#undef min
Copy link
Contributor

Choose a reason for hiding this comment

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

very minor :
I believe you can #undef unconditionally, whether min is already defined or not.
This is explicitly allowed by the standard,
and I believe no compiler warning will be triggered.

* Add `Portability.h` to fix min/max issues.
* Fix conversion warnings
* Assert that windowLog <= 23, which is currently always the case.
  This could be loosened, but we aren't looking to add new functionality.

Fixes on top of PR facebook#3375 by @eli-schwartz, which added Windows CI for contrib & programs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants