From 420d6302c31028f4cf615001e40662be897d67c6 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Sat, 30 Dec 2023 23:18:56 +0800
Subject: [PATCH] Simplify embassy run(), hide UnusedCli etc

They still need to be public for some reason, even
though they don't seem to be in input or output arguments.
---
 async/src/cmdline_client.rs   |  1 -
 embassy/src/client.rs         | 11 ++++++-----
 embassy/src/embassy_sunset.rs |  8 ++++++--
 embassy/src/server.rs         | 15 +++++++--------
 src/behaviour.rs              | 13 +++++++------
 src/channel.rs                |  1 +
 src/conn.rs                   |  1 +
 src/lib.rs                    |  2 +-
 src/runner.rs                 |  2 +-
 9 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/async/src/cmdline_client.rs b/async/src/cmdline_client.rs
index 2f1c062..9b81ebd 100644
--- a/async/src/cmdline_client.rs
+++ b/async/src/cmdline_client.rs
@@ -8,7 +8,6 @@ use core::fmt::Debug;
 use sunset::{AuthSigMsg, SignKey, OwnedSig, Pty, sshnames};
 use sunset::{BhError, BhResult};
 use sunset::{Error, Result, Runner, SessionCommand};
-use sunset::behaviour::{UnusedCli, UnusedServ};
 use sunset_embassy::*;
 
 use std::collections::VecDeque;
diff --git a/embassy/src/client.rs b/embassy/src/client.rs
index f472c8c..d72fb36 100644
--- a/embassy/src/client.rs
+++ b/embassy/src/client.rs
@@ -1,7 +1,6 @@
 use embedded_io_async::{Read, Write};
 
 use sunset::*;
-use sunset::behaviour::UnusedServ;
 
 use crate::*;
 use sunset::ChanData;
@@ -30,12 +29,14 @@ impl<'a> SSHClient<'a> {
         Ok(Self { sunset })
     }
 
-    pub async fn run<B: ?Sized, M: RawMutex, C: CliBehaviour>(&self,
+    /// Runs the session to completion.
+    ///
+    /// `rsock` and `wsock` are the SSH network channel (TCP port 22 or equivalent).
+    /// `b` is an instance of [`CliBehaviour`] which defines application behaviour.
+    pub async fn run<C: CliBehaviour>(&self,
         rsock: &mut impl Read,
         wsock: &mut impl Write,
-        b: &Mutex<M, B>) -> Result<()>
-        where
-            for<'f> Behaviour<'f, C, UnusedServ>: From<&'f mut B>
+        b: &SunsetMutex<C>) -> Result<()>
     {
         self.sunset.run(rsock, wsock, b).await
     }
diff --git a/embassy/src/embassy_sunset.rs b/embassy/src/embassy_sunset.rs
index 47ddd96..9a229c4 100644
--- a/embassy/src/embassy_sunset.rs
+++ b/embassy/src/embassy_sunset.rs
@@ -8,7 +8,7 @@ pub use defmt::{debug, error, info, panic, trace, warn};
 
 use core::future::{poll_fn, Future};
 use core::task::{Poll, Context};
-use core::ops::ControlFlow;
+use core::ops::{ControlFlow, DerefMut};
 use core::sync::atomic::AtomicBool;
 use core::sync::atomic::Ordering::{Relaxed, SeqCst};
 
@@ -310,8 +310,12 @@ impl<'a> EmbassySunset<'a> {
             let mut inner = self.inner.lock().await;
             {
                 {
+                    // lock the Mutex around the CliBehaviour or ServBehaviour
                     let mut b = b.lock().await;
-                    let b: &mut B = &mut b;
+                    // dereference the MutexGuard
+                    let b = b.deref_mut();
+                    // create either Behaviour<C, UnusedServ> or Behaviour<UnusedCli, S>
+                    // to pass to the runner.
                     let mut b: Behaviour<C, S> = b.into();
                     ret = inner.runner.progress(&mut b).await?;
                     // b is dropped, allowing other users
diff --git a/embassy/src/server.rs b/embassy/src/server.rs
index 88b1dc3..615c51e 100644
--- a/embassy/src/server.rs
+++ b/embassy/src/server.rs
@@ -1,9 +1,6 @@
-use embassy_sync::mutex::Mutex;
-use embassy_sync::blocking_mutex::raw::RawMutex;
 use embedded_io_async::{Read, Write};
 
 use sunset::*;
-use sunset::behaviour::UnusedCli;
 
 use crate::*;
 use embassy_sunset::EmbassySunset;
@@ -17,7 +14,7 @@ use embassy_sunset::EmbassySunset;
 /// and [`stdio_stderr()`][Self::stdio_stderr] methods.
 ///
 /// This is async executor agnostic, though requires the `ServBehaviour` instance
-/// to be wrapped in an Embassy [`Mutex`].
+/// to be wrapped in a [`SunsetMutex`].
 pub struct SSHServer<'a> {
     sunset: EmbassySunset<'a>,
 }
@@ -31,12 +28,14 @@ impl<'a> SSHServer<'a> {
         Ok(Self { sunset })
     }
 
-    pub async fn run<B: ?Sized, M: RawMutex, S: ServBehaviour>(&self,
+    /// Runs the session to completion.
+    ///
+    /// `rsock` and `wsock` are the SSH network channel (TCP port 22 or equivalent).
+    /// `b` is an instance of [`ServBehaviour`] which defines application behaviour.
+    pub async fn run<S: ServBehaviour>(&self,
         rsock: &mut impl Read,
         wsock: &mut impl Write,
-        b: &Mutex<M, B>) -> Result<()>
-        where
-            for<'f> Behaviour<'f, UnusedCli, S>: From<&'f mut B>
+        b: &SunsetMutex<S>) -> Result<()>
     {
         self.sunset.run(rsock, wsock, b).await
     }
diff --git a/src/behaviour.rs b/src/behaviour.rs
index 08fe38f..9b2ff61 100644
--- a/src/behaviour.rs
+++ b/src/behaviour.rs
@@ -4,6 +4,8 @@ use {
     log::{debug, error, info, log, trace, warn},
 };
 
+use core::convert::Infallible;
+
 use snafu::prelude::*;
 
 use crate::*;
@@ -296,10 +298,11 @@ pub trait ServBehaviour {
     }
 }
 
-/// A placeholder that will not be instantiated
+// Placeholders that will not be instantiated
+// For some reason these need to be public because they leak
+// from the async call to EmbassySunset::run() ?
 #[doc(hidden)]
-#[derive(Debug)]
-pub struct UnusedCli;
+pub struct UnusedCli(Infallible);
 impl CliBehaviour for UnusedCli {
     fn username(&mut self) -> BhResult<ResponseString> {
         unreachable!()
@@ -312,10 +315,8 @@ impl CliBehaviour for UnusedCli {
     }
 }
 
-/// A placeholder that will not be instantiated
 #[doc(hidden)]
-#[derive(Debug)]
-pub struct UnusedServ;
+pub struct UnusedServ(Infallible);
 impl ServBehaviour for UnusedServ {
     fn hostkeys(&mut self) -> BhResult<heapless::Vec<&SignKey, 2>> {
         unreachable!()
diff --git a/src/channel.rs b/src/channel.rs
index 2dff752..f307331 100644
--- a/src/channel.rs
+++ b/src/channel.rs
@@ -19,6 +19,7 @@ use sshnames::*;
 use sshwire::{BinString, TextString, SSHEncodeEnum};
 use traffic::TrafSend;
 use runner::ChanHandle;
+use behaviour::Behaviour;
 
 use snafu::ErrorCompat;
 
diff --git a/src/conn.rs b/src/conn.rs
index 16bd39c..1c7ee3a 100644
--- a/src/conn.rs
+++ b/src/conn.rs
@@ -21,6 +21,7 @@ use traffic::TrafSend;
 use channel::Channels;
 use config::MAX_CHANNELS;
 use kex::{Kex, SessId, AlgoConfig};
+use behaviour::Behaviour;
 
 /// The core state of a SSH instance.
 pub(crate) struct Conn {
diff --git a/src/lib.rs b/src/lib.rs
index 33d1018..f2d7fec 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -18,7 +18,7 @@ pub mod packets;
 pub mod sshnames;
 pub mod config;
 // exported so that UnusedCli can be used
-pub mod behaviour;
+mod behaviour;
 // exported so that some Channel error variants can be created with .fail().
 // perhaps the ones of interest should be expored separately.
 pub mod error;
diff --git a/src/runner.rs b/src/runner.rs
index 1bcde5b..4146868 100644
--- a/src/runner.rs
+++ b/src/runner.rs
@@ -13,7 +13,7 @@ use packets::{ChannelDataExt, ChannelData};
 use crate::channel::{ChanNum, ChanData};
 use encrypt::KeyState;
 use traffic::{TrafIn, TrafOut, TrafSend};
-use behaviour::{UnusedCli, UnusedServ};
+use behaviour::Behaviour;
 
 use conn::{Conn, Dispatched};
 
-- 
GitLab