Fix over-iterating buffer in hex string utility. #141
Conversation
|
Can one of the admins verify this patch? |
6 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
| import XCTest | ||
|
|
||
| // Not sure if NSString-bridged "String.init(format:_:)" is available on Linux/Windows. | ||
| #if (os(macOS) || os(iOS) || os(tvOS) || os(watchOS)) && CRYPTO_IN_SWIFTPM_FORCE_BUILD_API |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I'm happy with only testing the one in the tests module, tbh.
Fixes #139: over-iteration in hex string.
Checklist
If you've made changes to
gybfiles.script/generate_boilerplate_files_with_gyband included updated generated files in a commit of this pull requestMotivation:
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,hexStringshould return correct hex.