grpc-native: Address PR review issues

Signed-off-by: Johannes Zottele <[email protected]>

Author: Jozott00

grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/internal/KTag.kt

@@ -34,7 +34,7 @@ internal fun KTag.toRawKTag(): UInt {
     return (fieldNr.toUInt() shl K_TAG_TYPE_BITS) or wireType.ordinal.toUInt()
 }
 
-internal fun KTag.Companion.from(rawKTag: UInt): KTag? {
+internal fun KTag.Companion.fromOrNull(rawKTag: UInt): KTag? {
     val type = (rawKTag and K_TAG_TYPE_MASK).toInt()
     val field = (rawKTag shr K_TAG_TYPE_BITS).toInt()
     if (!isValidFieldNr(field)) {

grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/internal/WireDecoder.kt

@@ -9,6 +9,11 @@ import kotlinx.io.Buffer
 /**
  * A platform-specific decoder for wire format data.
  *
+ * This decoder is used by first calling [readTag], than looking up the field based on the field number in the returned,
+ * tag and then calling the actual `read*()` method to read the value to the corresponding field.
+ * This means that the nullable return value does not collide with optional fields, as optional fields would not
+ * include a tag in the encoded message.
+ *
  * If one `read*()` method returns `null`, decoding the data failed and no further
  * decoding can be done.
  *
@@ -43,6 +48,7 @@ internal interface WireDecoder : AutoCloseable {
     fun readSFixed64(): Long?
     fun readFloat(): Float?
     fun readDouble(): Double?
+
     fun readEnum(): Int?
     fun readString(): String?
     fun readBytes(): ByteArray?

grpc/grpc-core/src/jvmMain/java/kotlinx/rpc/grpc/internal/WireDecoder.jvm.kt

@@ -12,6 +12,7 @@ import kotlin.experimental.ExperimentalNativeApi
 import kotlin.math.min
 import kotlin.native.ref.createCleaner
 
+// TODO: Evaluate if this buffer size is suitable for all targets (KRPC-186)
 // maximum buffer size to allocate as contiguous memory in bytes
 private const val MAX_PACKED_BULK_SIZE: Int = 1_000_000
 
@@ -25,7 +26,7 @@ internal class WireDecoderNative(private val source: Buffer) : WireDecoder {
     // construct the pw_decoder_t by passing a pw_zero_copy_input_t that provides a bridge between
     // the CodedInputStream and the given source buffer. it passes functions that call the respective
     // ZeroCopyInputSource methods.
-    internal val raw = run {
+    internal val raw: CPointer<pw_decoder_t> = run {
         // construct the pw_zero_copy_input_t that functions as a bridge to the ZeroCopyInputSource
         val zeroCopyCInput = cValue<pw_zero_copy_input> {
             ctx = zeroCopyInput.asCPointer()
@@ -62,7 +63,7 @@ internal class WireDecoderNative(private val source: Buffer) : WireDecoder {
 
     override fun readTag(): KTag? {
         val tag = pw_decoder_read_tag(raw)
-        return KTag.from(tag)
+        return KTag.fromOrNull(tag)
     }
 
     override fun readBool(): Boolean? = memScoped {
@@ -177,7 +178,7 @@ internal class WireDecoderNative(private val source: Buffer) : WireDecoder {
         return null
     }
 
-    // TODO: Is it possible to avoid copying the c_str, by directly allocating a K/N String (as in readBytes)?
+    // TODO: Is it possible to avoid copying the c_str, by directly allocating a K/N String (as in readBytes)? KRPC-187
     override fun readString(): String? = memScoped {
         val str = alloc<CPointerVar<pw_string_t>>()
         val ok = pw_decoder_read_string(raw, str.ptr)
@@ -263,7 +264,7 @@ internal class WireDecoderNative(private val source: Buffer) : WireDecoder {
         val byteLen = readInt32() ?: return null
         if (byteLen < 0) return null
         if (source.size < byteLen) return null
-        if (byteLen == 0) return null
+        if (byteLen == 0) return emptyList()
 
         val limit = pw_decoder_push_limit(raw, byteLen)
 

grpc/grpc-core/src/jvmMain/kotlin/kotlinx/rpc/grpc/internal/WireDecoder.kt

@@ -30,7 +30,7 @@ internal class WireEncoderNative(private val sink: Sink) : WireEncoder {
     private val context = StableRef.create(this.Ctx())
 
     // construct encoder with a callback that calls write() on this.context
-    internal val raw = run {
+    internal val raw: CPointer<pw_encoder_t> = run {
         pw_encoder_new(context.asCPointer(), staticCFunction { ctx, buf, size ->
             if (buf == null || ctx == null) {
                 return@staticCFunction false
@@ -175,8 +175,7 @@ private inline fun <T> WireEncoderNative.writePackedInternal(
     fieldSize: Int,
     crossinline writer: (CValuesRef<pw_encoder_t>?, T) -> Boolean
 ): Boolean {
-    val ktag = KTag(fieldNr, WireType.LENGTH_DELIMITED).toRawKTag()
-    pw_encoder_write_tag(raw, ktag)
+    pw_encoder_write_tag(raw, fieldNr, WireType.LENGTH_DELIMITED.ordinal)
     // write the field size of the packed field
     pw_encoder_write_int32_no_tag(raw, fieldSize)
     for (v in value) {

grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/WireDecoder.native.kt

@@ -13,7 +13,7 @@ import platform.posix.memcpy
 
 
 @OptIn(ExperimentalForeignApi::class, InternalIoApi::class, UnsafeIoApi::class)
-public fun Sink.writeFully(buffer: CPointer<ByteVar>, offset: Long, length: Long) {
+internal fun Sink.writeFully(buffer: CPointer<ByteVar>, offset: Long, length: Long) {
     var consumed = 0L
     while (consumed < length) {
         UnsafeBufferOperations.writeToTail(this.buffer, 1) { array, start, endExclusive ->

grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/WireEncoder.native.kt

@@ -1,4 +1,3 @@
-load("@emsdk//emscripten_toolchain:wasm_rules.bzl", "wasm_cc_binary")
 load("@rules_cc//cc:defs.bzl", "cc_library")
 
 cc_binary(
@@ -24,7 +23,7 @@ cc_library(
     includes = ["include"],
     visibility = ["//visibility:public"],
     deps = [
-        # TODO: Reduce the dependencies and only use required once
+        # TODO: Reduce the dependencies and only use required once. KRPC-185
         "@com_github_grpc_grpc//:channelz",
         "@com_github_grpc_grpc//:generic_stub",
         "@com_github_grpc_grpc//:grpc++",

grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/bufferUnsafeExtensions.kt

@@ -22,13 +22,3 @@ bazel_dep(
     version = "1.73.1",
     repo_name = "com_github_grpc_grpc",
 )
-
-emsdk_version = "4.0.11"
-
-bazel_dep(name = "emsdk", version = emsdk_version)
-git_override(
-    module_name = "emsdk",
-    remote = "https://github.com/emscripten-core/emsdk.git",
-    strip_prefix = "bazel",
-    tag = emsdk_version,
-)