Remove workaround for JNA bug when passing small structs by value.

This (partially) reverts commit 937f9b7 The upstream JNA bug[1] has been resolved. JNA 5.7 includes the fix. We're upgrading our Docker test image to 5.8 already. Fixes mozilla#334 [1]: java-native-access/jna#1259
badboy · Aug 3, 2021 · 1e580ce · 1e580ce
1 parent 0a28de6
commit 1e580ce
Show file tree

Hide file tree

Showing 10 changed files with 10 additions and 52 deletions.
diff --git a/docker/Dockerfile-build b/docker/Dockerfile-build
@@ -66,7 +66,7 @@ RUN mkdir -p /tmp/setup-kotlin \
 
 RUN mkdir -p /tmp/setup-jna \
     && cd /tmp/setup-jna \
-    && curl -o jna.jar https://repo1.maven.org/maven2/net/java/dev/jna/jna/5.6.0/jna-5.6.0.jar \
+    && curl -o jna.jar https://repo1.maven.org/maven2/net/java/dev/jna/jna/5.8.0/jna-5.8.0.jar \
     # XXX TODO: should check a sha256sum or something here...
     && sudo mv jna.jar /opt \
     && echo "export CLASSPATH=\"\$CLASSPATH:/opt/jna.jar\"" >> /home/circleci/.bashrc \

diff --git a/uniffi/src/ffi/foreignbytes.rs b/uniffi/src/ffi/foreignbytes.rs
@@ -31,10 +31,6 @@ pub struct ForeignBytes {
     len: i32,
     /// The pointer to the foreign-owned bytes.
     data: *const u8,
-    /// This forces the struct to be larger than 16 bytes, as a temporary workaround for a bug in JNA.
-    /// See https://github.com/mozilla/uniffi-rs/issues/334 for details.
-    padding: i64,
-    padding2: i32,
 }
 
 impl ForeignBytes {
@@ -47,12 +43,7 @@ impl ForeignBytes {
     ///
     /// You must ensure that the raw parts uphold the documented invariants of this class.
     pub unsafe fn from_raw_parts(data: *const u8, len: i32) -> Self {
-        Self {
-            len,
-            data,
-            padding: 0,
-            padding2: 0,
-        }
+        Self { len, data }
     }
 
     /// View the foreign bytes as a `&[u8]`.

diff --git a/uniffi/src/ffi/rustbuffer.rs b/uniffi/src/ffi/rustbuffer.rs
@@ -58,9 +58,6 @@ pub struct RustBuffer {
     len: i32,
     /// The pointer to the allocated buffer of the `Vec<u8>`.
     data: *mut u8,
-    /// This forces the struct to be larger than 16 bytes, as a temporary workaround for a bug in JNA.
-    /// See https://github.com/mozilla/uniffi-rs/issues/334 for details.
-    padding: i64,
 }
 
 impl RustBuffer {
@@ -85,7 +82,6 @@ impl RustBuffer {
             capacity,
             len,
             data,
-            padding: 0,
         }
     }
 

diff --git a/uniffi_bindgen/src/bindings/gecko_js/templates/FFIDeclarationsTemplate.h b/uniffi_bindgen/src/bindings/gecko_js/templates/FFIDeclarationsTemplate.h
@@ -4,18 +4,11 @@ struct {{ context.ffi_rustbuffer_type() }} {
   int32_t mCapacity;
   int32_t mLen;
   uint8_t* mData;
-
-  // Ref https://github.com/mozilla/uniffi-rs/issues/334 re mPadding workaround
-  int64_t mPadding;
 };
 
 struct {{ context.ffi_foreignbytes_type() }} {
   int32_t mLen;
   const uint8_t* mData;
-
-  // Ref https://github.com/mozilla/uniffi-rs/issues/334 re padding workarounds
-  int64_t mPadding;
-  int32_t mPadding2;
 };
 
 struct {{ context.ffi_rustcallstatus_type() }} {

diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/RustBufferTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/RustBufferTemplate.kt
@@ -2,13 +2,11 @@
 // A rust-owned buffer is represented by its capacity, its current length, and a
 // pointer to the underlying data.
 
-@Structure.FieldOrder("capacity", "len", "data", "padding")
+@Structure.FieldOrder("capacity", "len", "data")
 open class RustBuffer : Structure() {
     @JvmField var capacity: Int = 0
     @JvmField var len: Int = 0
     @JvmField var data: Pointer? = null
-    // Ref https://github.com/mozilla/uniffi-rs/issues/334 for this weird "padding" field.
-    @JvmField var padding: Long = 0
 
     class ByValue : RustBuffer(), Structure.ByValue
     class ByReference : RustBuffer(), Structure.ByReference
@@ -40,13 +38,10 @@ open class RustBuffer : Structure() {
 // then we might as well copy it into a `RustBuffer`. But it's here for API
 // completeness.
 
-@Structure.FieldOrder("len", "data", "padding", "padding2")
+@Structure.FieldOrder("len", "data")
 open class ForeignBytes : Structure() {
     @JvmField var len: Int = 0
     @JvmField var data: Pointer? = null
-    // Ref https://github.com/mozilla/uniffi-rs/issues/334 for these weird "padding" fields.
-    @JvmField var padding: Long = 0
-    @JvmField var padding2: Int = 0
 
     class ByValue : ForeignBytes(), Structure.ByValue
 }

diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py
@@ -4,8 +4,6 @@ class RustBuffer(ctypes.Structure):
         ("capacity", ctypes.c_int32),
         ("len", ctypes.c_int32),
         ("data", ctypes.POINTER(ctypes.c_char)),
-        # Ref https://github.com/mozilla/uniffi-rs/issues/334 for this weird "padding" field.
-        ("padding", ctypes.c_int64),
     ]
 
     @staticmethod
@@ -179,9 +177,6 @@ class ForeignBytes(ctypes.Structure):
     _fields_ = [
         ("len", ctypes.c_int32),
         ("data", ctypes.POINTER(ctypes.c_char)),
-        # Ref https://github.com/mozilla/uniffi-rs/issues/334 for these weird "padding" fields.
-        ("padding", ctypes.c_int64),
-        ("padding2", ctypes.c_int32),
     ]
 
     def __str__(self):

diff --git a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb
@@ -1,9 +1,7 @@
 class RustBuffer < FFI::Struct
-  # Ref https://github.com/mozilla/uniffi-rs/issues/334 for this weird "padding" field.
   layout :capacity, :int32,
          :len,      :int32,
-         :data,     :pointer,
-         :padding,  :int64
+         :data,     :pointer
 
   def self.alloc(size)
     return {{ ci.namespace()|class_name_rb }}.rust_call(:{{ ci.ffi_rustbuffer_alloc().name() }}, size)
@@ -173,9 +171,7 @@ def consumeInto{{ canonical_type_name }}
 module UniFFILib
   class ForeignBytes < FFI::Struct
     layout :len,      :int32,
-           :data,     :pointer,
-           :padding,  :int64,
-           :padding2, :int32
+           :data,     :pointer
 
     def len
       self[:len]

diff --git a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h
@@ -26,17 +26,12 @@ typedef struct RustBuffer
     int32_t capacity;
     int32_t len;
     uint8_t *_Nullable data;
-    // Ref https://github.com/mozilla/uniffi-rs/issues/334 for this weird "padding" field.
-    int64_t padding;
 } RustBuffer;
 
 typedef struct ForeignBytes
 {
     int32_t len;
     const uint8_t *_Nullable data;
-    // Ref https://github.com/mozilla/uniffi-rs/issues/334 for these weird "padding" fields.
-    int64_t padding;
-    int32_t padding2;
 } ForeignBytes;
 
 // Error definitions
@@ -48,7 +43,7 @@ typedef struct RustCallStatus {
 // ⚠️ Attention: If you change this #else block (ending in `#endif // def UNIFFI_SHARED_H`) you *must* ⚠️
 // ⚠️ increment the version suffix in all instances of UNIFFI_SHARED_HEADER_V2 in this file.           ⚠️
 #endif // def UNIFFI_SHARED_H
-  
+
 {% for func in ci.iter_ffi_function_definitions() -%}
     {%- match func.return_type() -%}{%- when Some with (type_) %}{{ type_|type_ffi }}{% when None %}void{% endmatch %} {{ func.name() }}(
       {% call swift::arg_list_ffi_decl(func) %}

diff --git a/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift
@@ -35,8 +35,7 @@ fileprivate extension RustCallStatus {
             errorBuf: RustBuffer.init(
                 capacity: 0,
                 len: 0,
-                data: nil,
-                padding: 0
+                data: nil
             )
         )
     }

diff --git a/uniffi_bindgen/src/bindings/swift/templates/RustBufferTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/RustBufferTemplate.swift
@@ -4,8 +4,7 @@ fileprivate extension RustBuffer {
         let rbuf = bytes.withUnsafeBufferPointer { ptr in
             try! rustCall { {{ ci.ffi_rustbuffer_from_bytes().name() }}(ForeignBytes(bufferPointer: ptr), $0) }
         }
-        // Ref https://github.com/mozilla/uniffi-rs/issues/334 for the extra "padding" arg.
-        self.init(capacity: rbuf.capacity, len: rbuf.len, data: rbuf.data, padding: 0)
+        self.init(capacity: rbuf.capacity, len: rbuf.len, data: rbuf.data)
     }
 
     // Frees the buffer in place.
@@ -17,7 +16,6 @@ fileprivate extension RustBuffer {
 
 fileprivate extension ForeignBytes {
     init(bufferPointer: UnsafeBufferPointer<UInt8>) {
-        // Ref https://github.com/mozilla/uniffi-rs/issues/334 for the extra "padding" args.
-        self.init(len: Int32(bufferPointer.count), data: bufferPointer.baseAddress, padding: 0, padding2: 0)
+        self.init(len: Int32(bufferPointer.count), data: bufferPointer.baseAddress)
     }
 }