Fix over-iterating buffer in hex string utility.

[VaslD]

Fixes #139: over-iteration in hex string.

Checklist

• I've run tests to see all new and existing tests pass
• I've followed the code style of the rest of the project
• I've read the Contribution Guidelines
• I've updated the documentation if necessary

If you've made changes to gyb files

• I've run .script/generate_boilerplate_files_with_gyb and included updated generated files in a commit of this pull request

Motivation:

Upon discussing it in #139, an issue was identified that could cause unexpected hex output for a data buffer whose memory spans multiple non-contiguous regions.

Modifications:

Removed memory region specific logic. Fix applied to 3 copies of the same function.

Also added unit test on Apple platforms to compare with NSString's hex output.

Result:

With multi-region DataProtocols, hexString should return correct hex.

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[Lukasa]

Thanks for this! One minor tweak in the compiler defines and we should be go.

[Lukasa]

Sorry, this line needs to be:

#if (os(macOS) || os(iOS) || os(watchOS) || os(tvOS)) && CRYPTO_IN_SWIFTPM && !CRYPTO_IN_SWIFTPM_FORCE_BUILD_API
// Skip tests that require @testable imports of CryptoKit.
#else
#if (os(macOS) || os(iOS) || os(watchOS) || os(tvOS)) && !CRYPTO_IN_SWIFTPM_FORCE_BUILD_API
@testable import CryptoKit
#else
@testable import Crypto
#endif

[VaslD]

I did look at what other tests wrote. But I wasn't checking whether to use CryptoKit or Crypto, rather I needed to know (guess) whether String.init(format:_:) was available in the SDK. On appleOS this API is provided for sure by bridging from NSString's initializer, but on Linux and Windows I couldn't know.

The code you suggested would make this test always available cross-platform. Let me verify if Swift's cross-platform Foundation port has this API. Otherwise I'll need to do something like sprintf to make it cross-platform.

[VaslD]

Also CryptoTests defines its own copy of PrettyBytes. It's weird because I though @testable import means that test target has access to everything non-private in the original Crypto module.

This would mean the PrettyBytesTests does not actually need to import anything, as the test only tests functions defined in the test target itself.

Can you clarify this?

[Lukasa]

I'm happy with only testing the one in the tests module, tbh.