Refactor: remove once cell

Cargo.toml

@@ -22,7 +22,6 @@ toml = "0.8.19"
 signal-hook = "0.3.17"
 async-std = { version = "1.12.0", features = ["attributes"] }
 futures-util = { version = "0.3.30", default-features = false, features = ["io"] }
-once_cell = "1.19.0"
 libloading = "0.8.6"
 log = "0.4.25"
 simplelog = "0.12.2"

src/config.rs

@@ -1,10 +1,9 @@
 use async_std::{fs, io, sync::RwLock};
 use log::{info, warn};
-use once_cell::sync::OnceCell;
 use serde::{Deserialize, Serialize};
 
 use std::net::{IpAddr, Ipv4Addr};
-use std::{env, path::Path, path::PathBuf};
+use std::{env, path::Path, path::PathBuf, sync::OnceLock};
 
 use crate::HOME_DIR;
 use crate::args::Args;
@@ -42,7 +41,7 @@ const DEFAULT_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1));
 const DEFAULT_PORT: u16 = 6600;
 const DEFAULT_RETRIES: isize = 3;
 
-pub static CONFIG: OnceCell<RwLock<Config>> = OnceCell::new();
+pub static CONFIG: OnceLock<RwLock<Config>> = OnceLock::new();
 
 /// Returns a refrence to the global CONFIG value.
 ///
@@ -169,10 +168,10 @@ impl Config {
 }
 
 fn default_music_dir() -> PathBuf {
-    [&HOME_DIR, "Music"].iter().collect()
+    HOME_DIR.join("Music")
 }
 fn default_cover_dir() -> PathBuf {
-    [&HOME_DIR, "Music", "covers"].iter().collect()
+    HOME_DIR.join("Music/covers")
 }
 fn default_addr() -> IpAddr {
     DEFAULT_ADDR

src/main.rs

@@ -1,9 +1,8 @@
 use async_std::task::block_on;
 use libc::{EXIT_FAILURE, EXIT_SUCCESS, SIGHUP, SIGQUIT};
 use log::{debug, error, info, warn};
-use once_cell::sync::Lazy;
-use std::sync::{Arc, atomic::AtomicBool};
-use std::{env, io, process::exit};
+use std::sync::{Arc, LazyLock, atomic::AtomicBool};
+use std::{env, io, path::PathBuf, process::exit};
 
 use signal_hook::{consts::TERM_SIGNALS, flag, iterator::Signals, low_level::emulate_default_handler};
 
@@ -20,7 +19,7 @@ mod util;
 
 #[rustfmt::skip]
 const VERSION_STR: &str = concat!("Running ", env!("CARGO_BIN_NAME"), " v", env!("CARGO_PKG_VERSION"), " (", env!("GIT_HASH"), ") compiled using rustc v", env!("RUSTC_VERSION"));
-static HOME_DIR: Lazy<String> = Lazy::new(|| env::var("HOME").expect("$HOME must be set"));
+static HOME_DIR: LazyLock<PathBuf> = LazyLock::new(|| env::home_dir().expect("the user should always have a home dir"));
 
 #[cfg(target_os = "linux")]
 fn main() {

src/util/expand.rs

@@ -1,88 +1,74 @@
-use std::env;
+use std::{env, ffi::OsString, path::PathBuf};
 
 use log::warn;
 use serde::{Deserialize, Deserializer};
 
 use crate::HOME_DIR;
 
-pub fn serde_expand_path<'de, D: Deserializer<'de>, T: From<String>>(de: D) -> Result<T, D::Error> {
+pub fn serde_expand_path<'de, D: Deserializer<'de>, T: From<PathBuf>>(de: D) -> Result<T, D::Error> {
     Ok(expand_path(&String::deserialize(de)?).into())
 }
 
-pub fn expand_path(str: &str) -> String {
+pub fn expand_path(str: &str) -> PathBuf {
     if str == "~" {
         return HOME_DIR.clone();
-    } else if (!str.contains('$') && !str.contains('~')) || str.chars().count() <= 1 {
-        return str.to_string();
+    } else if (!str.contains('$') && !str.contains('~')) || str.chars().count() < 2 {
+        return PathBuf::from(str);
     }
 
-    let mut ret = String::with_capacity(str.len());
-
     // str has at least 2 chars because we checked it above
     let (first, second) = {
         let mut i = str.chars();
         (i.next().unwrap(), i.next().unwrap())
     };
 
-    if first == '~' && second == '/' {
-        ret.reserve(HOME_DIR.len());
-
-        // this isn't actually unsafe because the content of HOME_DIR is always valid UTF-8
-        unsafe {
-            ret.as_mut_vec().append(&mut HOME_DIR.clone().into_bytes());
-        }
-    }
+    let mut ret = if first == '~' && second == '/' {
+        let home = HOME_DIR.as_os_str();
+        let mut s = OsString::with_capacity(home.len() + str.len() - 2);
+        s.push(home);
+        s
+    } else {
+        OsString::with_capacity(str.len())
+    };
 
-    let mut remaining = if !ret.is_empty() { &str[1..] } else { str };
-    while let Some(dollar_idx) = remaining.find('$') {
-        ret.push_str(&remaining[..dollar_idx]);
+    let mut rest = if !ret.is_empty() { &str[1..] } else { str };
+    while let Some(dollar_idx) = rest.find('$') {
+        ret.push(&rest[..dollar_idx]);
 
-        remaining = &remaining[dollar_idx + 1..];
+        rest = &rest[dollar_idx + 1..];
 
         // if varname empty ignore it
-        if remaining.len() <= 1 || !is_valid_varname_char(remaining.as_bytes()[0] as char) {
-            ret.push('$');
+        if rest.len() <= 1 || !is_valid_varname_char(rest.as_bytes()[0] as char) {
+            ret.push("$");
             continue;
         }
 
         // if the dollar sign is escaped ignore it
-        if is_char_escaped(&str.as_bytes()[..dollar_idx]) {
-            ret.push('$');
+        if is_char_escaped(&str[..dollar_idx]) {
+            ret.push("$");
             continue;
         }
 
-        // go from dollar-idx until non-varname char
-        let mut end_idx = remaining.len() - 1;
-        for (i, chr) in remaining.chars().enumerate() {
-            if !is_valid_varname_char(chr) {
-                end_idx = i - 1;
-                break;
-            }
-        }
+        let end_idx = rest.chars().position(|b| !is_valid_varname_char(b)).unwrap_or(rest.len());
 
-        let varname = &remaining[..=end_idx];
-        match env::var(varname) {
-            Ok(var) => {
-                ret.reserve(var.len());
-                // this isn't actually unsafe because the content of var is always valid UTF-8
-                unsafe {
-                    ret.as_mut_vec().append(&mut var.into_bytes());
-                }
-            }
-            Err(_e) => {
+        let varname = &rest[..end_idx];
+
+        match env::var_os(varname) {
+            Some(var) => ret.push(var),
+            None => {
                 warn!("encountered undefined environment variable: {varname}");
+
                 ret.reserve(varname.len() + 1);
-                ret.push('$');
-                ret.push_str(varname);
+                ret.push("$");
+                ret.push(varname);
             }
         }
 
-        remaining = &remaining[end_idx + 1..];
+        rest = &rest[end_idx..];
     }
 
-    ret.push_str(remaining);
-
-    ret
+    ret.push(rest);
+    PathBuf::from(ret)
 }
 
 fn is_valid_varname_char(chr: char) -> bool {
@@ -92,42 +78,42 @@ fn is_valid_varname_char(chr: char) -> bool {
 /// Checks if a char is backslash escaped by looking at the chars before it.<br />
 /// E.g. "\$" -> true; "\\$" -> false; "\\\$" -> true
 ///
-/// the first byte of bytes should be the index after the char to check
-fn is_char_escaped(bytes: &[u8]) -> bool {
-    if bytes.is_empty() {
+/// the last char of s should be the char before the possibly escaped char.
+fn is_char_escaped(s: &str) -> bool {
+    if s.is_empty() {
         return false;
     }
 
     let mut escaped = false;
-    let mut n = bytes.len();
-    while n > 0 {
-        if bytes[n - 1] != b'\\' {
+    for chr in s.chars().rev() {
+        if chr != '\\' {
             break;
         }
 
         escaped = !escaped;
-        n -= 1;
     }
 
     escaped
 }
 
 #[cfg(test)]
 mod tests {
+    use std::path::Path;
+
     use super::*;
 
     #[test]
     fn test_escaped_char() {
-        assert!(is_char_escaped(r"pr\\efi\x\".as_bytes()));
-        assert!(is_char_escaped(r"\".as_bytes()));
-        assert!(is_char_escaped(r"\postfix\".as_bytes()));
-        assert!(is_char_escaped(r"\\postfix\".as_bytes()));
-        assert!(!is_char_escaped(r"\postfix".as_bytes()));
-        assert!(!is_char_escaped(r"\\".as_bytes()));
-        assert!(!is_char_escaped(r"\\\\".as_bytes()));
-        assert!(!is_char_escaped(r"\\middle\\".as_bytes()));
-        assert!(!is_char_escaped(r"\\middle\f\\\\".as_bytes()));
-        assert!(!is_char_escaped(&[]));
+        assert!(is_char_escaped(r"pr\\efi\x\"));
+        assert!(is_char_escaped(r"\"));
+        assert!(is_char_escaped(r"\postfix\"));
+        assert!(is_char_escaped(r"\\postfix\"));
+        assert!(!is_char_escaped(r"\postfix"));
+        assert!(!is_char_escaped(r"\\"));
+        assert!(!is_char_escaped(r"\\\\"));
+        assert!(!is_char_escaped(r"\\middle\\"));
+        assert!(!is_char_escaped(r"\\middle\f\\\\"));
+        assert!(!is_char_escaped(""));
     }
 
     #[test]
@@ -136,40 +122,43 @@ mod tests {
             env::remove_var("UNSET_VAR");
         }
 
-        assert_eq!(expand_path("some/path"), "some/path");
-        assert_eq!(expand_path("$UNSET_VAR/some/path"), "$UNSET_VAR/some/path");
-        assert_eq!(expand_path("/some/$UNSET_VAR/path"), "/some/$UNSET_VAR/path");
-        assert_eq!(expand_path("/some/path/$UNSET_VAR"), "/some/path/$UNSET_VAR");
-        assert_eq!(expand_path("/some/path$"), "/some/path$");
-        assert_eq!(expand_path("$/some/path"), "$/some/path");
-        assert_eq!(expand_path("$"), "$");
+        assert_eq!(expand_path("some/path"), Path::new("some/path"));
+        assert_eq!(expand_path("$UNSET_VAR/some/path"), Path::new("$UNSET_VAR/some/path"));
+        assert_eq!(expand_path("/some/$UNSET_VAR/path"), Path::new("/some/$UNSET_VAR/path"));
+        assert_eq!(expand_path("/some/path/$UNSET_VAR"), Path::new("/some/path/$UNSET_VAR"));
+        assert_eq!(expand_path("/some/path$"), Path::new("/some/path$"));
+        assert_eq!(expand_path("$/some/path"), Path::new("$/some/path"));
+        assert_eq!(expand_path("$"), Path::new("$"));
     }
 
     #[test]
     fn test_expansion() {
         unsafe {
-            env::set_var("HOME", "/home/repeatable");
-            env::set_var("SOME_VAR", "relative");
+            env::set_var("HOME", Path::new("/home/repeatable"));
+            env::set_var("SOME_VAR", Path::new("relative"));
             env::remove_var("UNSET_VAR");
         }
 
-        assert_eq!(expand_path("~"), "/home/repeatable");
-        assert_eq!(expand_path("~/"), "/home/repeatable/");
-        assert_eq!(expand_path("/some/file/path/~/"), "/some/file/path/~/");
-        assert_eq!(expand_path("~/some/dir/names"), "/home/repeatable/some/dir/names");
-        assert_eq!(expand_path("~/ continue"), "/home/repeatable/ continue");
-        assert_eq!(expand_path("~$UNSET_VAR"), "~$UNSET_VAR");
-        assert_eq!(expand_path("~abcdef"), "~abcdef");
-        assert_eq!(expand_path("~~"), "~~");
-        assert_eq!(expand_path("~_"), "~_");
-
-        assert_eq!(expand_path("$HOME"), "/home/repeatable");
-        assert_eq!(expand_path("$HOME-"), "/home/repeatable-");
-        assert_eq!(expand_path("$HOME_/path"), "$HOME_/path");
-        assert_eq!(expand_path("$HOME/$SOME_VAR/dir"), "/home/repeatable/relative/dir");
-        assert_eq!(expand_path("$SOME_VAR/"), "relative/");
-        assert_eq!(expand_path("/some/path/$SOME_VAR-SOME_VAR"), "/some/path/relative-SOME_VAR");
-        assert_eq!(expand_path(r"/some/path/\$HOME"), r"/some/path/\$HOME");
-        assert_eq!(expand_path("/some/path/$HOME_HOME"), "/some/path/$HOME_HOME");
+        assert_eq!(expand_path("~"), Path::new("/home/repeatable"));
+        assert_eq!(expand_path("~/"), Path::new("/home/repeatable/"));
+        assert_eq!(expand_path("/some/file/path/~/"), Path::new("/some/file/path/~/"));
+        assert_eq!(expand_path("~/some/dir/names"), Path::new("/home/repeatable/some/dir/names"));
+        assert_eq!(expand_path("~/ continue"), Path::new("/home/repeatable/ continue"));
+        assert_eq!(expand_path("~$UNSET_VAR"), Path::new("~$UNSET_VAR"));
+        assert_eq!(expand_path("~abcdef"), Path::new("~abcdef"));
+        assert_eq!(expand_path("~~"), Path::new("~~"));
+        assert_eq!(expand_path("~_"), Path::new("~_"));
+
+        assert_eq!(expand_path("$HOME"), Path::new("/home/repeatable"));
+        assert_eq!(expand_path("$HOME-"), Path::new("/home/repeatable-"));
+        assert_eq!(expand_path("$HOME_/path"), Path::new("$HOME_/path"));
+        assert_eq!(expand_path("$HOME/$SOME_VAR/dir"), Path::new("/home/repeatable/relative/dir"));
+        assert_eq!(expand_path("$SOME_VAR/"), Path::new("relative/"));
+        assert_eq!(
+            expand_path("/some/path/$SOME_VAR-SOME_VAR"),
+            Path::new("/some/path/relative-SOME_VAR")
+        );
+        assert_eq!(expand_path(r"/some/path/\$HOME"), Path::new(r"/some/path/\$HOME"));
+        assert_eq!(expand_path("/some/path/$HOME_HOME"), Path::new("/some/path/$HOME_HOME"));
     }
 }

src/util/mod.rs

@@ -12,11 +12,8 @@ pub mod notify;
 /// Defined as: $XDG_CONFIG_PATH/mpdris/mpdris.conf or $HOME/.config/mpdris/mpdris.conf
 /// Deprecated path: $XDG_CONFIG_PATH/mpd/mpDris.conf or $HOME/.config/mpd/mpDris.conf
 pub fn get_config_path() -> PathBuf {
-    let base = env::var("XDG_CONFIG_HOME").unwrap_or_else(|_| format!("{}/.config", *HOME_DIR));
-    let paths: [PathBuf; 2] = [
-        [base.as_str(), "mpdris", "mpdris.conf"].iter().collect(),
-        [base.as_str(), "mpd", "mpDris.conf"].iter().collect(),
-    ];
+    let base = env::var_os("XDG_CONFIG_HOME").map_or_else(|| HOME_DIR.join(".config"), PathBuf::from);
+    let paths = [base.join("mpdris/mpdris.conf"), base.join("mpd/mpDris.conf")];
     let idx = paths.iter().position(|p| p.is_file()).unwrap_or(0);
     if idx == 1 {
         warn!("Using deprecated configuration path `{}`", paths[idx].display());