From 4126617d5d5e088e81c877eaf6f2b2fc392018db Mon Sep 17 00:00:00 2001
From: Nick Fitzgerald <fitzgen@gmail.com>
Date: Fri, 10 Jul 2020 12:03:03 -0700
Subject: [PATCH] wasmtime-c-api: Make `wasm_table_set` *not* take ownership of
 its reference

Same for `wasm_table_grow` and `wasm_table_new` and their `init` values.
---
 crates/c-api/include/doc-wasm.h |  6 +++++-
 crates/c-api/src/ref.rs         | 11 -----------
 crates/c-api/src/table.rs       | 29 ++++++++++++++++-------------
 examples/externref.c            |  2 +-
 4 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/crates/c-api/include/doc-wasm.h b/crates/c-api/include/doc-wasm.h
index 51da740f8661..096c92d2e1c2 100644
--- a/crates/c-api/include/doc-wasm.h
+++ b/crates/c-api/include/doc-wasm.h
@@ -1764,6 +1764,8 @@
  * Returns an error if the #wasm_ref_t does not match the element type of the
  * table provided or if it comes from a different store than the one provided.
  *
+ * Does not take ownship of the `init` value.
+ *
  * > Note: for funcref tables you can use #wasmtime_funcref_table_new as well.
  *
  * \fn wasm_tabletype_t *wasm_table_type(const wasm_table_t *);
@@ -1792,7 +1794,7 @@
  * * The #wasm_ref_t comes from a different store than the table provided.
  * * The #wasm_ref_t does not have an appropriate type to store in this table.
  *
- * Takes ownership of the given `wasm_ref_t*`.
+ * Does not take ownership of the given `wasm_ref_t*`.
  *
  * > Note: for funcref tables you can use #wasmtime_funcref_table_set to learn
  * > about errors.
@@ -1813,6 +1815,8 @@
  * * The #wasm_ref_t comes from a different store than the table provided.
  * * The #wasm_ref_t does not have an appropriate type to store in this table.
  *
+ * Does not take ownership of the givein `init` value.
+ *
  * > Note: for funcref tables you can use #wasmtime_funcref_table_grow as well.
  */
 
diff --git a/crates/c-api/src/ref.rs b/crates/c-api/src/ref.rs
index a6ffef23046f..890a6e96e819 100644
--- a/crates/c-api/src/ref.rs
+++ b/crates/c-api/src/ref.rs
@@ -30,17 +30,6 @@ pub(crate) enum WasmRefInner {
 
 wasmtime_c_api_macros::declare_own!(wasm_ref_t);
 
-pub(crate) fn ref_into_val(r: Option<Box<wasm_ref_t>>) -> Option<Val> {
-    // Let callers decide whether to treat this as a null `funcref` or a
-    // null `externref`.
-    let r = r?;
-
-    Some(match r.r {
-        WasmRefInner::ExternRef(x) => Val::ExternRef(Some(x)),
-        WasmRefInner::FuncRef(f) => Val::FuncRef(Some(f)),
-    })
-}
-
 pub(crate) fn ref_to_val(r: &wasm_ref_t) -> Val {
     match &r.r {
         WasmRefInner::ExternRef(x) => Val::ExternRef(Some(x.clone())),
diff --git a/crates/c-api/src/table.rs b/crates/c-api/src/table.rs
index 29a675ba5390..c88620da8587 100644
--- a/crates/c-api/src/table.rs
+++ b/crates/c-api/src/table.rs
@@ -1,4 +1,4 @@
-use crate::r#ref::{ref_into_val, val_into_ref};
+use crate::r#ref::{ref_to_val, val_into_ref};
 use crate::{handle_result, wasm_func_t, wasm_ref_t, wasmtime_error_t};
 use crate::{wasm_extern_t, wasm_store_t, wasm_tabletype_t};
 use std::ptr;
@@ -30,21 +30,24 @@ impl wasm_table_t {
     }
 }
 
-fn ref_into_val_for_table(r: Option<Box<wasm_ref_t>>, table_ty: &TableType) -> Val {
-    ref_into_val(r).unwrap_or_else(|| match table_ty.element() {
-        ValType::FuncRef => Val::FuncRef(None),
-        ValType::ExternRef => Val::ExternRef(None),
-        ty => panic!("unsupported table element type: {:?}", ty),
-    })
+fn ref_to_val_for_table(r: Option<&wasm_ref_t>, table_ty: &TableType) -> Val {
+    r.map_or_else(
+        || match table_ty.element() {
+            ValType::FuncRef => Val::FuncRef(None),
+            ValType::ExternRef => Val::ExternRef(None),
+            ty => panic!("unsupported table element type: {:?}", ty),
+        },
+        |r| ref_to_val(r),
+    )
 }
 
 #[no_mangle]
 pub extern "C" fn wasm_table_new(
     store: &wasm_store_t,
     tt: &wasm_tabletype_t,
-    init: Option<Box<wasm_ref_t>>,
+    init: Option<&wasm_ref_t>,
 ) -> Option<Box<wasm_table_t>> {
-    let init = ref_into_val_for_table(init, &tt.ty().ty);
+    let init = ref_to_val_for_table(init, &tt.ty().ty);
     let table = Table::new(&store.store, tt.ty().ty.clone(), init).ok()?;
     Some(Box::new(wasm_table_t {
         ext: wasm_extern_t {
@@ -115,9 +118,9 @@ pub extern "C" fn wasmtime_funcref_table_get(
 pub unsafe extern "C" fn wasm_table_set(
     t: &wasm_table_t,
     index: wasm_table_size_t,
-    r: Option<Box<wasm_ref_t>>,
+    r: Option<&wasm_ref_t>,
 ) -> bool {
-    let val = ref_into_val_for_table(r, &t.table().ty());
+    let val = ref_to_val_for_table(r, &t.table().ty());
     t.table().set(index, val).is_ok()
 }
 
@@ -143,9 +146,9 @@ pub extern "C" fn wasm_table_size(t: &wasm_table_t) -> wasm_table_size_t {
 pub unsafe extern "C" fn wasm_table_grow(
     t: &wasm_table_t,
     delta: wasm_table_size_t,
-    init: Option<Box<wasm_ref_t>>,
+    init: Option<&wasm_ref_t>,
 ) -> bool {
-    let init = ref_into_val_for_table(init, &t.table().ty());
+    let init = ref_to_val_for_table(init, &t.table().ty());
     t.table().grow(delta, init).is_ok()
 }
 
diff --git a/examples/externref.c b/examples/externref.c
index 5bbe35f42d63..0305fc9999fe 100644
--- a/examples/externref.c
+++ b/examples/externref.c
@@ -108,9 +108,9 @@ int main() {
   assert(elem.kind == WASM_ANYREF);
   ok = wasm_table_set(table, 3, elem.of.ref);
   assert(ok);
-  elem.of.ref = NULL;
 
   // `table[3]` should now be our `externref`.
+  wasm_ref_delete(elem.of.ref);
   elem.of.ref = wasm_table_get(table, 3);
   assert(elem.of.ref != NULL);
   assert(wasm_ref_same(elem.of.ref, externref.of.ref));