microsoft / krabsetw

KrabsETW provides a modern C++ wrapper and a .NET wrapper around the low-level ETW trace consumption functions.
Other
588 stars 147 forks source link

Replace "min" by "std::min<size_t>" to support NOMINMAX #162

Closed pierricgimmig closed 3 years ago

pierricgimmig commented 3 years ago

Including krabsetw from a project that defines "NOMINMAX" would cause a compilation error due to the absence of the "min" macro. The issue is fixed by using "std::min" instead of "min". Note that we explicitly specify the template type in "std::min" to avoid a clash with the "min" macro in the case where "NOMINMAX" is not defined.

Also, check that there is no truncation when casting size_t to USHORT.

Tests: ran all test.

ghost commented 3 years ago

CLA assistant check
All CLA requirements met.

pierricgimmig commented 3 years ago

Thanks for doing this! If you are importing krabs via the nuget then please update the version numbers in the three .nuspec files at the root of the repo. Otherwise you are good to go!

Great! My pleasure! I'm not using nuget but I bumped the version in the three .nuspec files.

Also, it seems I can't merge the PR myself as "the base branch restricts merging to authorized users".

Thanks!

swannman commented 3 years ago

@pierricgimmig thanks for updating the .nuspec files! Looks like signing the CLA is the last step and then we can merge this.

pierricgimmig commented 3 years ago

Ah yes of course, done. Thanks!