-
Notifications
You must be signed in to change notification settings - Fork 3
kn: checksum bindings #121
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
Changes from all commits
63ac5a4
9938e2b
6044551
b668b29
943ee32
8fc034a
e4f58d6
5757c98
8a5669a
c21bbe4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| package aws.sdk.kotlin.crt | ||
|
|
||
| /** | ||
| * A mixin class used to ensure CRT is initialized before the class is invoked | ||
| */ | ||
| public open class WithCrt { | ||
| init { | ||
| CRT.initRuntime { } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,9 @@ | |
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| package aws.sdk.kotlin.crt.util.hashing | ||
|
|
||
| import aws.sdk.kotlin.crt.WithCrt | ||
| import kotlinx.cinterop.* | ||
| import libcrt.aws_checksums_crc32 | ||
| import libcrt.aws_checksums_crc32c | ||
|
|
@@ -21,6 +24,10 @@ internal class Crc(val checksumFn: AwsChecksumsCrcFunction) : HashFunction { | |
| private var crc = 0U | ||
|
|
||
| override fun update(input: ByteArray, offset: Int, length: Int) { | ||
| if (input.isEmpty() || length == 0) { | ||
| return | ||
| } | ||
|
|
||
|
Comment on lines
+27
to
+30
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Why is this necessary? What breaks if we just pass an empty blob or a 0 length down to CRT?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The request would never make it to CRT, the following code snippet fails with an |
||
| val offsetInput = input.usePinned { | ||
| it.addressOf(offset) | ||
| } | ||
|
|
@@ -42,7 +49,9 @@ internal class Crc(val checksumFn: AwsChecksumsCrcFunction) : HashFunction { | |
| /** | ||
| * A CRC32 [HashFunction] implemented using bindings to CRT. | ||
| */ | ||
| public class Crc32 : HashFunction { | ||
| public class Crc32 : | ||
| WithCrt(), | ||
| HashFunction { | ||
| private val crc32 = Crc(::aws_checksums_crc32) | ||
| override fun update(input: ByteArray, offset: Int, length: Int) { | ||
| crc32.update(input, offset, length) | ||
|
|
@@ -56,7 +65,9 @@ public class Crc32 : HashFunction { | |
| /** | ||
| * A CRC32C [HashFunction] implemented using bindings to CRT. | ||
| */ | ||
| public class Crc32c : HashFunction { | ||
| public class Crc32c : | ||
| WithCrt(), | ||
| HashFunction { | ||
| private val crc32c = Crc(::aws_checksums_crc32c) | ||
| override fun update(input: ByteArray, offset: Int, length: Int) { | ||
| crc32c.update(input, offset, length) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| package aws.sdk.kotlin.crt.util.hashing | ||
|
|
||
| import aws.sdk.kotlin.crt.Allocator | ||
| import aws.sdk.kotlin.crt.WithCrt | ||
| import aws.sdk.kotlin.crt.awsAssertOpSuccess | ||
| import kotlinx.cinterop.addressOf | ||
| import kotlinx.cinterop.cValue | ||
| import kotlinx.cinterop.convert | ||
| import kotlinx.cinterop.pointed | ||
| import kotlinx.cinterop.reinterpret | ||
| import kotlinx.cinterop.usePinned | ||
| import libcrt.aws_byte_buf | ||
| import libcrt.aws_byte_cursor_from_array | ||
| import libcrt.aws_hash_destroy | ||
| import libcrt.aws_hash_finalize | ||
| import libcrt.aws_hash_update | ||
| import libcrt.aws_md5_new | ||
|
|
||
| /** | ||
| * MD5 hash function implemented using bindings to CRT | ||
| */ | ||
| public class Md5 : | ||
| WithCrt(), | ||
| HashFunction { | ||
| private var md5 = checkNotNull(aws_md5_new(Allocator.Default)) { "aws_md5_new" } | ||
|
|
||
| override fun update(input: ByteArray, offset: Int, length: Int) { | ||
| if (input.isEmpty() || length == 0) { | ||
| return | ||
| } | ||
|
|
||
| val inputCursor = input.usePinned { | ||
| aws_byte_cursor_from_array(it.addressOf(offset), length.convert()) | ||
| } | ||
|
|
||
| awsAssertOpSuccess(aws_hash_update(md5, inputCursor)) { | ||
| "aws_hash_update" | ||
| } | ||
| } | ||
|
|
||
| override fun digest(): ByteArray { | ||
| val output = ByteArray(md5.pointed.digest_size.toInt()) | ||
|
|
||
| val byteBuf = output.usePinned { | ||
| cValue<aws_byte_buf> { | ||
| capacity = output.size.convert() | ||
| len = 0U | ||
| buffer = it.addressOf(0).reinterpret() | ||
| } | ||
| } | ||
|
|
||
| awsAssertOpSuccess(aws_hash_finalize(md5, byteBuf, 0U)) { "aws_hash_finalize" } | ||
|
|
||
| return output.also { reset() } | ||
| } | ||
|
|
||
| override fun reset() { | ||
| aws_hash_destroy(md5) | ||
| md5 = checkNotNull(aws_md5_new(Allocator.Default)) { "aws_md5_new" } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: I'm concerned with this approach for two reasons:
Crc32but never uses any methods on it, we'll've wasted cycles unnecessarily.Could we move the initialization closer to the site of use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we'll just have to take care to initialize CRT everywhere we use it, which we currently don't do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is true but in practice CRT is only initialized once, subsequent calls to
CRT.initRuntime { }are no-ops. I'm assuming users of aws-sdk-kotlin Kotlin/Native will need to initialize CRT at some pointThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We spoke offline and agreed to continue this approach for now. Ideally we would create a singleton object
CrtLiband use cinterop to generate extensions for it (likeCrtLib.aws_checksums_crc32(...)). ThisCrtLibobject would ensureCRT.initRuntime { ... }is called before invoking the actual C function.