Clean up a bit

4 files changed, 244 insertions, 174 deletions

diff --git a/src/eat.rs b/src/eat.rs
new file mode 100644
index 0000000..6591836
--- /dev/null
+++ b/src/eat.rs
@@ -0,0 +1,34 @@
+// This file is part of Hexchat Plugin API Bindings for Rust
+// Copyright (C) 2018, 2021 Soni L.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as
+// published by the Free Software Foundation, either version 3 of the
+// License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+/// A status indicator for event callbacks. Indicates whether to continue processing, eat hexchat,
+/// eat plugin, or eat all.
+///
+/// Returned by most hooks.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+pub struct Eat {
+    pub(crate) do_eat: i32,
+}
+
+/// Equivalent to HEXCHAT_EAT_NONE.
+pub const EAT_NONE: Eat = Eat { do_eat: 0 };
+/// Equivalent to HEXCHAT_EAT_HEXCHAT.
+pub const EAT_HEXCHAT: Eat = Eat { do_eat: 1 };
+/// Equivalent to HEXCHAT_EAT_PLUGIN.
+pub const EAT_PLUGIN: Eat = Eat { do_eat: 2 };
+/// Equivalent to HEXCHAT_EAT_ALL.
+pub const EAT_ALL: Eat = Eat { do_eat: 1 | 2 };
+
diff --git a/src/infoid.rs b/src/infoid.rs
new file mode 100644
index 0000000..d39f059
--- /dev/null
+++ b/src/infoid.rs
@@ -0,0 +1,85 @@
+// This file is part of Hexchat Plugin API Bindings for Rust
+// Copyright (C) 2018, 2021 Soni L.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as
+// published by the Free Software Foundation, either version 3 of the
+// License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+use std::borrow::Cow;
+
+/// A hexchat_get_info key.
+#[derive(Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Copy, Clone)]
+pub enum InfoId<'a> {
+    /// Returns the away message, or `None` if the user is not away.
+    Away,
+    /// Returns the current channel name.
+    Channel,
+    /// Returns the current charset.
+    Charset,
+    /// Returns the hexchat configuration directory, e.g. `/home/user/.config/hexchat`.
+    Configdir,
+    /// Returns the text event format string for the given text event name.
+    EventText(&'a str),
+    /// Returns the (real) hostname of the current server.
+    Host,
+    /// Returns the contents of the input box.
+    Inputbox,
+    // TODO replace with a get_libdirfs function!
+    // /// Returns the library directory, e.g. `/usr/lib/hexchat`.
+    // ///
+    // /// May not always work, as this string isn't necessarily UTF-8, but local file system
+    // /// encoding.
+    // Libdirfs,
+    /// Returns the channel modes, if known, or `None`.
+    Modes,
+    /// Returns the current network name, or `None`.
+    Network,
+    /// Returns the user's current nick.
+    Nick,
+    /// Returns the user's nickserv password, if any, or `None`
+    Nickserv,
+    /// Returns the current server name, or `None` if you are not connected.
+    Server,
+    /// Returns the current channel topic.
+    Topic,
+    /// Returns the HexChat version string.
+    Version,
+    /// Returns the window status: "active", "hidden" or "normal".
+    WinStatus,
+}
+
+impl<'a> InfoId<'a> {
+    pub fn name(&self) -> Cow<'static, str> {
+        match *self {
+            InfoId::EventText(s) => {
+                let mut eventtext: String = "event_text ".into();
+                eventtext.push_str(&s);
+                eventtext.into()
+            },
+            InfoId::Away      => "away".into(),
+            InfoId::Channel   => "channel".into(),
+            InfoId::Charset   => "charset".into(),
+            InfoId::Configdir => "configdir".into(),
+            InfoId::Host      => "host".into(),
+            InfoId::Inputbox  => "inputbox".into(),
+            //InfoId::Libdirfs  => "libdirfs".into(),
+            InfoId::Modes     => "modes".into(),
+            InfoId::Network   => "network".into(),
+            InfoId::Nick      => "nick".into(),
+            InfoId::Nickserv  => "nickserv".into(),
+            InfoId::Server    => "server".into(),
+            InfoId::Topic     => "topic".into(),
+            InfoId::Version   => "version".into(),
+            InfoId::WinStatus => "win_status".into(),
+        }
+    }
+}
diff --git a/src/lib.rs b/src/lib.rs
index 465ff5e..b64cac4 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -158,13 +158,21 @@
 #[doc(hidden)]
 pub extern crate libc;
 
+mod eat;
+mod infoid;
 mod internals;
+mod pluginfo;
+
+pub use eat::*;
+pub use infoid::InfoId;
+
+use pluginfo::PluginInfo;
 
-use std::borrow::Cow;
 use std::cell::Cell;
 use std::ffi::{CString, CStr};
 use std::marker::PhantomData;
 use std::mem;
+use std::mem::ManuallyDrop;
 use std::ops;
 use std::panic::catch_unwind;
 use std::ptr;
@@ -180,16 +188,6 @@ use std::time::{SystemTime, UNIX_EPOCH, Duration};
 
 // Consts
 
-// EAT_*
-/// Equivalent to HEXCHAT_EAT_NONE.
-pub const EAT_NONE: Eat = Eat { do_eat: 0 };
-/// Equivalent to HEXCHAT_EAT_HEXCHAT.
-pub const EAT_HEXCHAT: Eat = Eat { do_eat: 1 };
-/// Equivalent to HEXCHAT_EAT_PLUGIN.
-pub const EAT_PLUGIN: Eat = Eat { do_eat: 2 };
-/// Equivalent to HEXCHAT_EAT_ALL.
-pub const EAT_ALL: Eat = Eat { do_eat: 1 | 2 };
-
 // PRI_*
 /// Equivalent to HEXCHAT_PRI_HIGHEST
 pub const PRI_HIGHEST: i32 = 127;
@@ -212,7 +210,8 @@ pub trait Plugin {
     /// Called to deinitialize the plugin.
     ///
     /// This is always called immediately prior to Drop::drop.
-    fn deinit(&self, _ph: &mut PluginHandle) {
+    fn deinit(&self, ph: &mut PluginHandle) {
+        let _ = ph;
     }
 }
 
@@ -305,15 +304,6 @@ pub struct EventAttrs<'a> {
     _dummy: PhantomData<&'a ()>,
 }
 
-/// A status indicator for event callbacks. Indicates whether to continue processing, eat hexchat,
-/// eat plugin, or eat all.
-///
-/// Returned by most hooks.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
-pub struct Eat {
-    do_eat: i32,
-}
-
 /// A command hook handle.
 #[must_use = "Hooks must be stored somewhere and are automatically unhooked on Drop"]
 pub struct CommandHookHandle {
@@ -364,77 +354,10 @@ pub struct InvalidContextError<F: FnOnce(EnsureValidContext) -> R, R>(F);
 
 // Enums
 
-/// A hexchat_get_info key.
-#[derive(Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Copy, Clone)]
-pub enum InfoId<'a> {
-    /// Returns the away message, or `None` if the user is not away.
-    Away,
-    /// Returns the current channel name.
-    Channel,
-    /// Returns the current charset.
-    Charset,
-    /// Returns the hexchat configuration directory, e.g. `/home/user/.config/hexchat`.
-    Configdir,
-    /// Returns the text event format string for the given text event name.
-    EventText(&'a str),
-    /// Returns the (real) hostname of the current server.
-    Host,
-    /// Returns the contents of the input box.
-    Inputbox,
-    /// Returns the library directory, e.g. `/usr/lib/hexchat`.
-    ///
-    /// May not always work, as this string isn't necessarily UTF-8, but local file system
-    /// encoding.
-    Libdirfs,
-    /// Returns the channel modes, if known, or `None`.
-    Modes,
-    /// Returns the current network name, or `None`.
-    Network,
-    /// Returns the user's current nick.
-    Nick,
-    /// Returns the user's nickserv password, if any, or `None`
-    Nickserv,
-    /// Returns the current server name, or `None` if you are not connected.
-    Server,
-    /// Returns the current channel topic.
-    Topic,
-    /// Returns the HexChat version string.
-    Version,
-    /// Returns the window status: "active", "hidden" or "normal".
-    WinStatus,
-}
-
 // ***** //
 // impls //
 // ***** //
 
-impl<'a> InfoId<'a> {
-    pub fn name(&self) -> Cow<'static, str> {
-        match *self {
-            InfoId::EventText(s) => {
-                let mut eventtext: String = "event_text ".into();
-                eventtext.push_str(&s);
-                eventtext.into()
-            },
-            InfoId::Away      => "away".into(),
-            InfoId::Channel   => "channel".into(),
-            InfoId::Charset   => "charset".into(),
-            InfoId::Configdir => "configdir".into(),
-            InfoId::Host      => "host".into(),
-            InfoId::Inputbox  => "inputbox".into(),
-            InfoId::Libdirfs  => "libdirfs".into(),
-            InfoId::Modes     => "modes".into(),
-            InfoId::Network   => "network".into(),
-            InfoId::Nick      => "nick".into(),
-            InfoId::Nickserv  => "nickserv".into(),
-            InfoId::Server    => "server".into(),
-            InfoId::Topic     => "topic".into(),
-            InfoId::Version   => "version".into(),
-            InfoId::WinStatus => "win_status".into(),
-        }
-    }
-}
-
 impl<F: FnOnce(EnsureValidContext) -> R, R> InvalidContextError<F, R> {
     pub fn get_closure(self) -> F {
         self.0
@@ -961,10 +884,7 @@ impl PluginHandle {
     }
 
     /// Returns information on the current context.
-    ///
-    /// Note: `InfoId::Libdirfs` may return `None` or broken results if the result wouldn't be (valid) UTF-8.
-    // TODO this should be `id: InfoId`. fix in 0.3
-    pub fn get_info(&mut self, id: &InfoId) -> Option<String> {
+    pub fn get_info(&mut self, id: InfoId) -> Option<String> {
         let ph = self.ph;
         let id_cstring = CString::new(&*id.name()).unwrap();
         unsafe {
@@ -973,11 +893,7 @@ impl PluginHandle {
                 None
             } else {
                 let s = CStr::from_ptr(res).to_owned().into_string();
-                if *id != InfoId::Libdirfs {
-                    Some(s.expect("non-utf8 word - broken hexchat"))
-                } else {
-                    s.ok()
-                }
+                Some(s.expect("non-utf8 word - broken hexchat"))
             }
         }
     }
@@ -1206,7 +1122,7 @@ impl<'a> EnsureValidContext<'a> {
     pub fn hook_timer<F>(&mut self, timeout: i32, cb: F) -> TimerHookHandle where F: Fn(&mut PluginHandle) -> bool + 'static + ::std::panic::RefUnwindSafe {
         self.ph.hook_timer(timeout, cb)
     }
-    pub fn get_info(&mut self, id: &InfoId) -> Option<String> {
+    pub fn get_info(&mut self, id: InfoId) -> Option<String> {
         self.ph.get_info(id)
     }
 }
@@ -1252,12 +1168,8 @@ fn cstr(b: &'static [u8]) -> *const libc::c_char {
 ///
 /// This function is unsafe because `ptr` must hame come from `Rc::into_raw`.
 unsafe fn rc_clone_from_raw<T>(ptr: *const T) -> Rc<T> {
-    // this is a bit confusing to read, but basically, we get an Rc from the raw ptr, and increment
-    // the refcount.
-    // The construct mem::forget(rc.clone()) increments the refcount.
-    let rc = Rc::from_raw(ptr);
-    mem::forget(rc.clone());
-    rc
+    let rc = ManuallyDrop::new(Rc::from_raw(ptr));
+    Rc::clone(&rc)
 }
 
 /// Converts a **valid** context pointer into a Context (Rust-managed) struct.
@@ -1330,67 +1242,6 @@ impl Drop for HexchatEventAttrsHelper {
     }
 }
 
-/// Holds name, desc, vers
-// This is kinda naughty - we modify these values after returning from hexchat_plugin_init, during
-// the deinitialization.
-// However, if my reading of the HexChat code is correct, this is "ok".
-#[derive(Copy, Clone)]
-struct PluginInfo {
-    name: *mut *const libc::c_char,
-    desc: *mut *const libc::c_char,
-    vers: *mut *const libc::c_char,
-}
-
-impl PluginInfo {
-    /// Creates a PluginInfo.
-    ///
-    /// # Panics
-    ///
-    /// This function explicitly doesn't panic. Call unwrap() on the result instead.
-    fn new(name: *mut *const libc::c_char, desc: *mut *const libc::c_char, vers: *mut *const libc::c_char) -> Option<PluginInfo> {
-        if name.is_null() || desc.is_null() || vers.is_null() || name == desc || desc == vers || name == vers {
-            None
-        } else {
-            Some(unsafe { PluginInfo::new_unchecked(name, desc, vers) })
-        }
-    }
-
-    /// Creates a PluginInfo without checking the arguments.
-    ///
-    /// # Safety
-    ///
-    /// This function is unsafe, as it doesn't check the validity of the arguments. You're expected
-    /// to only pass in non-aliased non-null pointers. Use new if unsure.
-    unsafe fn new_unchecked(name: *mut *const libc::c_char, desc: *mut *const libc::c_char, vers: *mut *const libc::c_char) -> PluginInfo {
-        PluginInfo {
-            name, desc, vers
-        }
-    }
-
-    /// Drop relevant CStrings.
-    ///
-    /// # Safety
-    ///
-    /// This function is unsafe because calling it may trigger Undefined Behaviour. See also
-    /// [CString::from_raw].
-    ///
-    /// [from_raw]: https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw
-    unsafe fn drop_info(&mut self) {
-        if !(*self.name).is_null() {
-            mem::drop(CString::from_raw(*self.name as *mut _));
-            *self.name = cstr(EMPTY_CSTRING_DATA);
-        }
-        if !(*self.desc).is_null() {
-            mem::drop(CString::from_raw(*self.desc as *mut _));
-            *self.desc = cstr(EMPTY_CSTRING_DATA);
-        }
-        if !(*self.vers).is_null() {
-            mem::drop(CString::from_raw(*self.vers as *mut _));
-            *self.vers = cstr(EMPTY_CSTRING_DATA);
-        }
-    }
-}
-
 /// Plugin data stored in the hexchat plugin_handle.
 struct PhUserdata {
     plug: Box<dyn Plugin>,
@@ -1404,12 +1255,22 @@ struct PhUserdata {
 /// This function is unsafe because it doesn't check if the pointer is valid.
 ///
 /// Improper use of this function can leak memory.
-unsafe fn put_userdata(ph: *mut internals::Ph, ud: Box<PhUserdata>) {
-    (*ph).userdata = Box::into_raw(ud) as *mut libc::c_void;
+unsafe fn put_userdata(ph: *mut internals::Ph, ud: Rc<PhUserdata>) {
+    (*ph).userdata = Rc::into_raw(ud) as *mut libc::c_void;
 }
 
-// unsafe fn get_userdata(ph: *mut internals::Ph) -> *const PhUserdata {
-//     (*ph).userdata as *const _
+// /// Clones the userdata from the plugin handle.
+// /// 
+// /// # Safety
+// ///
+// /// This function is unsafe because it doesn't check if the pointer is valid.
+// unsafe fn clone_userdata(ph: *mut internals::Ph) -> Option<Rc<PhUserdata>> {
+//     let ptr = (*ph).userdata as *const PhUserdata;
+//     if ptr.is_null() {
+//         None
+//     } else {
+//         Some(rc_clone_from_raw(ptr))
+//     }
 // }
 
 /// Pops the userdata from the plugin handle.
@@ -1417,8 +1278,8 @@ unsafe fn put_userdata(ph: *mut internals::Ph, ud: Box<PhUserdata>) {
 /// # Safety
 ///
 /// This function is unsafe because it doesn't check if the pointer is valid.
-unsafe fn pop_userdata(ph: *mut internals::Ph) -> Box<PhUserdata> {
-    Box::from_raw(mem::replace(&mut (*ph).userdata, ptr::null_mut()) as *mut PhUserdata)
+unsafe fn pop_userdata(ph: *mut internals::Ph) -> Rc<PhUserdata> {
+    Rc::from_raw(mem::replace(&mut (*ph).userdata, ptr::null_mut()) as *mut PhUserdata)
 }
 
 // *********************** //
@@ -1455,6 +1316,8 @@ pub unsafe fn hexchat_plugin_init<T>(plugin_handle: *mut libc::c_void,
         // no filename specified for some reason, but we can still load
         String::new() // empty string
     };
+    // TODO use filename
+    let _ = filename;
     // these may be null, unless initialization is successful.
     // we set them to null as markers.
     *plugin_name = ptr::null();
@@ -1484,13 +1347,13 @@ pub unsafe fn hexchat_plugin_init<T>(plugin_handle: *mut libc::c_void,
     } else {
         return 0;
     };
-    let r: thread::Result<Option<Box<_>>> = {
+    let r: thread::Result<Option<Rc<_>>> = {
         catch_unwind(move || {
             let mut pluginhandle = PluginHandle::new(ph, pluginfo);
             let plug = T::default();
             if plug.init(&mut pluginhandle, if !arg.is_null() { Some(CStr::from_ptr(arg).to_str().expect("arg not valid utf-8 - broken hexchat")) } else { None }) {
                 if !(pluginfo.name.is_null() || pluginfo.desc.is_null() || pluginfo.vers.is_null()) {
-                    Some(Box::new(PhUserdata { plug: Box::new(plug), pluginfo }))
+                    Some(Rc::new(PhUserdata { plug: Box::new(plug), pluginfo }))
                 } else {
                     // TODO log: forgot to call register
                     None
@@ -1538,7 +1401,7 @@ pub unsafe fn hexchat_plugin_deinit<T>(plugin_handle: *mut libc::c_void) -> libc
                 {
                     let mut ausinfo = ::std::panic::AssertUnwindSafe(&mut info);
                     safe_to_unload = if catch_unwind(move || {
-                        let userdata = *pop_userdata(ph);
+                        let userdata = pop_userdata(ph);
                         **ausinfo = Some(userdata.pluginfo);
                         userdata.plug.deinit(&mut PluginHandle::new(ph, userdata.pluginfo));
                     }).is_ok() { 1 } else { 0 };
diff --git a/src/pluginfo.rs b/src/pluginfo.rs
new file mode 100644
index 0000000..840dd2c
--- /dev/null
+++ b/src/pluginfo.rs
@@ -0,0 +1,88 @@
+// This file is part of Hexchat Plugin API Bindings for Rust
+// Copyright (C) 2018, 2021 Soni L.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as
+// published by the Free Software Foundation, either version 3 of the
+// License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+use crate::EMPTY_CSTRING_DATA;
+use crate::cstr;
+
+use std::mem;
+use std::ffi::CString;
+
+/// Holds name, desc, vers
+// This is kinda naughty - we modify these values after returning from hexchat_plugin_init, during
+// the deinitialization.
+// However, if my reading of the HexChat code is correct, this is "ok".
+#[derive(Copy, Clone)]
+pub struct PluginInfo {
+    pub name: *mut *const libc::c_char,
+    pub desc: *mut *const libc::c_char,
+    pub vers: *mut *const libc::c_char,
+}
+
+impl PluginInfo {
+    /// Creates a PluginInfo.
+    ///
+    /// # Safety
+    ///
+    /// This function is unsafe, as it can't guarantee the validity of its arguments. It does more
+    /// checking than `new_unchecked` however.
+    ///
+    /// # Panics
+    ///
+    /// This function explicitly doesn't panic. Call unwrap() on the result instead.
+    pub unsafe fn new(name: *mut *const libc::c_char, desc: *mut *const libc::c_char, vers: *mut *const libc::c_char) -> Option<PluginInfo> {
+        if name.is_null() || desc.is_null() || vers.is_null() || name == desc || desc == vers || name == vers {
+            None
+        } else {
+            Some(PluginInfo::new_unchecked(name, desc, vers))
+        }
+    }
+
+    /// Creates a PluginInfo without checking the arguments.
+    ///
+    /// # Safety
+    ///
+    /// This function is unsafe, as it doesn't check the validity of the arguments. You're expected
+    /// to only pass in non-aliased non-null pointers. Use new if unsure.
+    pub unsafe fn new_unchecked(name: *mut *const libc::c_char, desc: *mut *const libc::c_char, vers: *mut *const libc::c_char) -> PluginInfo {
+        PluginInfo {
+            name, desc, vers
+        }
+    }
+
+    /// Drop relevant CStrings.
+    ///
+    /// # Safety
+    ///
+    /// This function is unsafe because calling it may trigger Undefined Behaviour. See also
+    /// [CString::from_raw].
+    ///
+    /// [from_raw]: https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw
+    pub unsafe fn drop_info(&mut self) {
+        if !(*self.name).is_null() {
+            mem::drop(CString::from_raw(*self.name as *mut _));
+            *self.name = cstr(EMPTY_CSTRING_DATA);
+        }
+        if !(*self.desc).is_null() {
+            mem::drop(CString::from_raw(*self.desc as *mut _));
+            *self.desc = cstr(EMPTY_CSTRING_DATA);
+        }
+        if !(*self.vers).is_null() {
+            mem::drop(CString::from_raw(*self.vers as *mut _));
+            *self.vers = cstr(EMPTY_CSTRING_DATA);
+        }
+    }
+}
+