introduce MIRI tests, fix unsafe causing UB (#95)

Lots of unsafe code moving
ldesgoui · Mar 26, 2020 · b344458 · b344458
1 parent 2b18c2c
commit b344458
Show file tree

Hide file tree

Showing 25 changed files with 1,302 additions and 820 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -94,12 +94,40 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --target=${{ matrix.target }}
+          args: --target=${{ matrix.target }} -- --test-threads=1
         env:
           DISCORD_GAME_SDK_PATH: ${{ runner.temp }}
 
+  miri-test:
+    name: Test with MIRI
+
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v1
+
+      - name: Set up Rust
+        uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: nightly
+          override: true
+          components: miri
+
+      - name: Set up MIRI
+        uses: actions-rs/cargo@v1
+        with:
+          command: miri
+          args: setup
+
+      - name: Test with MIRI
+        run: |
+          cd discord_game_sdk
+          cargo miri test --features doc
+
   fmt-lint:
-    name: Check format, lints
+    name: Check format and lints
 
     runs-on: ubuntu-latest
 

diff --git a/discord_game_sdk/src/callback.rs b/discord_game_sdk/src/callback.rs
diff --git a/discord_game_sdk/src/discord.rs b/discord_game_sdk/src/discord.rs
@@ -1,4 +1,4 @@
-use crate::sys;
+use crate::{sys, ClientID};
 use std::{cell::UnsafeCell, marker::PhantomData, mem::ManuallyDrop};
 
 /// Main interface with SDK
@@ -70,6 +70,21 @@ impl<E> Drop for Discord<'_, E> {
 }
 
 impl<'d, E> Discord<'d, E> {
+    /// The Client ID that was supplied during creation
+    pub fn client_id(&self) -> ClientID {
+        self.inner().client_id
+    }
+
+    /// The [`EventHandler`](trait.EventHandler.html)
+    pub fn event_handler(&self) -> &Option<E> {
+        self.inner().event_handler()
+    }
+
+    /// The [`EventHandler`](trait.EventHandler.html)
+    pub fn event_handler_mut(&mut self) -> &mut Option<E> {
+        self.inner_mut().event_handler_mut()
+    }
+
     pub(crate) fn inner(&self) -> &DiscordInner<'d, E> {
         unsafe { &*self.0 }
     }

diff --git a/discord_game_sdk/src/lib.rs b/discord_game_sdk/src/lib.rs
@@ -114,14 +114,10 @@
 
 #![doc(html_root_url = "https://docs.rs/discord_game_sdk/1.0.0-rc.3")]
 
-#[macro_use]
-mod macros;
-
 mod action;
 mod activity;
 mod activity_kind;
 mod aliases;
-pub(crate) mod callback;
 mod cast;
 mod comparison;
 mod create_flags;
@@ -176,8 +172,13 @@ mod methods {
     mod store;
     mod users;
     mod voice;
+
+    mod callback;
 }
 
+#[cfg(test)]
+mod mock;
+
 pub(crate) use discord_game_sdk_sys as sys;
 
 pub use self::{

diff --git a/discord_game_sdk/src/lobby_member_transaction.rs b/discord_game_sdk/src/lobby_member_transaction.rs
@@ -1,4 +1,4 @@
-use crate::{sys, to_result::ToResult, utils, Result};
+use crate::{sys, to_result::ToResult, Result};
 use std::collections::HashMap;
 
 /// Lobby Member Transaction
@@ -27,11 +27,11 @@ impl LobbyMemberTransaction {
     pub fn add_metadata(&mut self, mut key: String, mut value: String) -> &mut Self {
         if !key.ends_with('\0') {
             key.push('\0')
-        };
+        }
 
         if !value.ends_with('\0') {
             value.push('\0')
-        };
+        }
 
         let _ = self.metadata.insert(key, Some(value));
 
@@ -48,35 +48,34 @@ impl LobbyMemberTransaction {
     pub fn delete_metadata<S>(&mut self, mut key: String) -> &mut Self {
         if !key.ends_with('\0') {
             key.push('\0')
-        };
+        }
 
         let _ = self.metadata.insert(key, None);
         self
     }
 
-    pub(crate) fn process(&self, ptr: *mut sys::IDiscordLobbyMemberTransaction) -> Result<()> {
+    pub(crate) unsafe fn process(
+        &self,
+        tx: *mut sys::IDiscordLobbyMemberTransaction,
+    ) -> Result<()> {
         for (key, value) in &self.metadata {
             match value {
                 Some(value) => {
-                    utils::with_tx(ptr, |tx| unsafe {
-                        tx.set_metadata.unwrap()(
-                            tx,
-                            // XXX: *mut should be *const
-                            key.as_ptr() as *mut u8,
-                            // XXX: *mut should be *const
-                            value.as_ptr() as *mut u8,
-                        )
-                    })
+                    (*tx).set_metadata.unwrap()(
+                        tx,
+                        // XXX: *mut should be *const
+                        key.as_ptr() as *mut u8,
+                        // XXX: *mut should be *const
+                        value.as_ptr() as *mut u8,
+                    )
                     .to_result()?;
                 }
                 None => {
-                    utils::with_tx(ptr, |tx| unsafe {
-                        tx.delete_metadata.unwrap()(
-                            tx,
-                            // XXX: *mut should be *const
-                            key.as_ptr() as *mut u8,
-                        )
-                    })
+                    (*tx).delete_metadata.unwrap()(
+                        tx,
+                        // XXX: *mut should be *const
+                        key.as_ptr() as *mut u8,
+                    )
                     .to_result()?;
                 }
             }

diff --git a/discord_game_sdk/src/lobby_transaction.rs b/discord_game_sdk/src/lobby_transaction.rs
@@ -1,4 +1,4 @@
-use crate::{sys, to_result::ToResult, utils, LobbyKind, Result, UserID};
+use crate::{sys, to_result::ToResult, LobbyKind, Result, UserID};
 use std::collections::HashMap;
 
 /// Lobby Transaction
@@ -56,11 +56,11 @@ impl LobbyTransaction {
     pub fn add_metadata(&mut self, mut key: String, mut value: String) -> &mut Self {
         if !key.ends_with('\0') {
             key.push('\0')
-        };
+        }
 
         if !value.ends_with('\0') {
             value.push('\0')
-        };
+        }
 
         let _ = self.metadata.insert(key, Some(value));
         self
@@ -76,7 +76,7 @@ impl LobbyTransaction {
     pub fn delete_metadata<S>(&mut self, mut key: String) -> &mut Self {
         if !key.ends_with('\0') {
             key.push('\0')
-        };
+        }
 
         let _ = self.metadata.insert(key, None);
         self
@@ -90,48 +90,42 @@ impl LobbyTransaction {
         self
     }
 
-    pub(crate) fn process(&self, ptr: *mut sys::IDiscordLobbyTransaction) -> Result<()> {
+    pub(crate) unsafe fn process(&self, tx: *mut sys::IDiscordLobbyTransaction) -> Result<()> {
         if let Some(kind) = self.kind {
-            utils::with_tx(ptr, |tx| unsafe { tx.set_type.unwrap()(tx, kind.into()) })
-                .to_result()?;
+            (*tx).set_type.unwrap()(tx, kind.into()).to_result()?;
         }
 
         if let Some(user_id) = self.owner {
-            utils::with_tx(ptr, |tx| unsafe { tx.set_owner.unwrap()(tx, user_id) }).to_result()?;
+            (*tx).set_owner.unwrap()(tx, user_id).to_result()?;
         }
 
         if let Some(capacity) = self.capacity {
-            utils::with_tx(ptr, |tx| unsafe { tx.set_capacity.unwrap()(tx, capacity) })
-                .to_result()?;
+            (*tx).set_capacity.unwrap()(tx, capacity).to_result()?;
         }
 
         if let Some(locked) = self.locked {
-            utils::with_tx(ptr, |tx| unsafe { tx.set_locked.unwrap()(tx, locked) }).to_result()?;
+            (*tx).set_locked.unwrap()(tx, locked).to_result()?;
         }
 
         for (key, value) in &self.metadata {
             match value {
                 Some(value) => {
-                    utils::with_tx(ptr, |tx| unsafe {
-                        tx.set_metadata.unwrap()(
-                            tx,
-                            // XXX: *mut should be *const
-                            key.as_ptr() as *mut u8,
-                            // XXX: *mut should be *const
-                            value.as_ptr() as *mut u8,
-                        )
-                    })
+                    (*tx).set_metadata.unwrap()(
+                        tx,
+                        // XXX: *mut should be *const
+                        key.as_ptr() as *mut u8,
+                        // XXX: *mut should be *const
+                        value.as_ptr() as *mut u8,
+                    )
                     .to_result()?;
                 }
 
                 None => {
-                    utils::with_tx(ptr, |tx| unsafe {
-                        tx.delete_metadata.unwrap()(
-                            tx,
-                            // XXX: *mut should be *const
-                            key.as_ptr() as *mut u8,
-                        )
-                    })
+                    (*tx).delete_metadata.unwrap()(
+                        tx,
+                        // XXX: *mut should be *const
+                        key.as_ptr() as *mut u8,
+                    )
                     .to_result()?;
                 }
             }

diff --git a/discord_game_sdk/src/macros.rs b/discord_game_sdk/src/macros.rs