Merge pull request #18 from snipsco/fix_cdrop

Fix double free for non-pointer type
sonos · Mar 19, 2020 · 26785ee · 26785ee
2 parents 52782a9 + df307de
commit 26785ee
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 13 deletions.
diff --git a/ffi-convert-derive/Cargo.toml b/ffi-convert-derive/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "ffi-convert-derive"
-version = "0.1.0"
+version = "0.1.1"
 authors = ["Sonos"]
 edition = "2018"
 license = "MIT OR Apache-2.0"

diff --git a/ffi-convert-derive/src/cdrop.rs b/ffi-convert-derive/src/cdrop.rs
@@ -19,12 +19,11 @@ pub fn impl_cdrop_macro(input: &syn::DeriveInput) -> TokenStream {
 
             let drop_field = if field.is_string {
                 quote!(ffi_convert::take_back_c_string!(self.#field_name))
+            } else if field.is_pointer {
+                quote!( unsafe { #field_type::drop_raw_pointer(self.#field_name) }? )
             } else {
-                if field.is_pointer {
-                    quote!( unsafe { #field_type::drop_raw_pointer(self.#field_name) }? )
-                } else {
-                    quote!( self.# field_name.do_drop()? )
-                }
+                // the other cases will be handled automatically by rust
+                quote!()
             };
 
             let conversion = if field.is_nullable {

diff --git a/ffi-convert-tests/Cargo.toml b/ffi-convert-tests/Cargo.toml
@@ -1,10 +1,10 @@
 [package]
 name = "ffi-convert-tests"
-version = "0.1.0"
+version = "0.1.1"
 authors = ["Sonos"]
 edition = "2018"
 
 [dependencies]
 failure = "0.1"
-ffi-convert = "0.1"
+ffi-convert = "0.1.1"
 libc = "0.2.66"
diff --git a/ffi-convert-tests/src/lib.rs b/ffi-convert-tests/src/lib.rs
@@ -104,13 +104,15 @@ pub struct CLayer {
 #[derive(Clone, Debug, PartialEq)]
 pub struct Dummy {
     pub count: i32,
+    pub describe: String,
 }
 
 #[repr(C)]
 #[derive(CReprOf, AsRust, CDrop)]
 #[target_type(Dummy)]
 pub struct CDummy {
     count: i32,
+    describe: *const libc::c_char,
 }
 
 #[cfg(test)]
@@ -123,7 +125,12 @@ mod tests {
         Topping { amount: 2 }
     });
 
-    generate_round_trip_rust_c_rust!(round_trip_dummy, Dummy, CDummy, { Dummy { count: 2 } });
+    generate_round_trip_rust_c_rust!(round_trip_dummy, Dummy, CDummy, {
+        Dummy {
+            count: 2,
+            describe: "yo".to_string(),
+        }
+    });
 
     generate_round_trip_rust_c_rust!(round_trip_layer, Layer, CLayer, {
         Layer {
@@ -138,7 +145,10 @@ mod tests {
             description: Some("I'm delicious ! ".to_string()),
             start: 0.0,
             end: Some(2.0),
-            dummy: Dummy { count: 2 },
+            dummy: Dummy {
+                count: 2,
+                describe: "yo".to_string(),
+            },
             sauce: Some(Sauce { volume: 32.23 }),
             toppings: vec![Topping { amount: 2 }, Topping { amount: 3 }],
             layers: Some(vec![Layer {
@@ -155,7 +165,10 @@ mod tests {
             description: Some("I'm delicious ! ".to_string()),
             start: 0.0,
             end: None,
-            dummy: Dummy { count: 2 },
+            dummy: Dummy {
+                count: 2,
+                describe: "yo".to_string(),
+            },
             sauce: None,
             toppings: vec![],
             layers: Some(vec![]),

diff --git a/ffi-convert/Cargo.toml b/ffi-convert/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "ffi-convert"
-version = "0.1.0"
+version = "0.1.1"
 authors = ["Sonos"]
 edition = "2018"
 license = "MIT OR Apache-2.0"
@@ -10,6 +10,6 @@ readme = "../README.md"
 keywords = ["ffi"]
 
 [dependencies]
-ffi-convert-derive = "0.1"
+ffi-convert-derive = "0.1.1"
 failure = "0.1"
 libc = "0.2"