Make wdirname and wbasename go &wstr -> &wstr

There is no reason for either of these functions to allocate, so have
them not do it.

fish-rust/src/builtins/status.rs

@@ -457,10 +457,10 @@ pub fn status(
                 STATUS_BASENAME | STATUS_DIRNAME | STATUS_FILENAME => {
                     let res = parser.current_filename_ffi().from_ffi();
                     let f = match (res.is_empty(), s) {
-                        (false, STATUS_DIRNAME) => wdirname(res),
-                        (false, STATUS_BASENAME) => wbasename(res),
-                        (true, _) => wgettext!("Standard input").to_owned(),
-                        (false, _) => res,
+                        (false, STATUS_DIRNAME) => wdirname(&res),
+                        (false, STATUS_BASENAME) => wbasename(&res),
+                        (true, _) => wgettext!("Standard input"),
+                        (false, _) => &res,
                     };
                     streams.out.append(wgettext_fmt!("%ls\n", f));
                 }

fish-rust/src/io.rs

@@ -646,10 +646,10 @@ impl IoChain {
                                 // or there's a non-directory component,
                                 // find the first problematic component for a better message.
                                 if [ENOENT, ENOTDIR].contains(&err) {
-                                    let mut dname = spec.target.clone();
+                                    let mut dname: &wstr = &spec.target;
                                     while !dname.is_empty() {
-                                        let next = wdirname(dname.clone());
-                                        if let Some(md) = wstat(&next) {
+                                        let next: &wstr = wdirname(dname);
+                                        if let Some(md) = wstat(next) {
                                             if !md.is_dir() {
                                                 FLOGF!(
                                                     warning,

fish-rust/src/path.rs

@@ -312,7 +312,7 @@ fn path_get_path_core<S: AsRef<wstr>>(cmd: &wstr, pathsv: &[S]) -> GetPathResult
                     // Keep the first *interesting* error and path around.
                     // ENOENT isn't interesting because not having a file is the normal case.
                     // Ignore if the parent directory is already inaccessible.
-                    if waccess(&wdirname(proposed_path.clone()), X_OK) == 0 {
+                    if waccess(wdirname(&proposed_path), X_OK) == 0 {
                         best = GetPathResult::new(Some(err), proposed_path);
                     }
                 }
@@ -642,8 +642,8 @@ fn create_directory(d: &wstr) -> bool {
         }
         None => {
             if errno().0 == ENOENT {
-                let dir = wdirname(d);
-                if create_directory(&dir) && wmkdir(d, 0o700) == 0 {
+                let dir: &wstr = wdirname(d);
+                if create_directory(dir) && wmkdir(d, 0o700) == 0 {
                     return true;
                 }
             }

fish-rust/src/wutil/mod.rs

@@ -322,80 +322,71 @@ pub fn path_normalize_for_cd(wd: &wstr, path: &wstr) -> WString {
 }
 
 /// Wide character version of dirname().
-#[widestrs]
-pub fn wdirname(path: impl AsRef<wstr>) -> WString {
-    let path = path.as_ref();
+pub fn wdirname(mut path: &wstr) -> &wstr {
     // Do not use system-provided dirname (#7837).
     // On Mac it's not thread safe, and will error for paths exceeding PATH_MAX.
     // This follows OpenGroup dirname recipe.
 
     // 1: Double-slash stays.
-    if path == "//"L {
-        return path.to_owned();
+    if path == "//" {
+        return path;
     }
 
     // 2: All slashes => return slash.
-    if !path.is_empty() && path.chars().find(|&c| c != '/').is_none() {
-        return "/"L.to_owned();
+    if !path.is_empty() && path.chars().all(|c| c == '/') {
+        return L!("/");
     }
 
     // 3: Trim trailing slashes.
-    let mut path = path.to_owned();
     while path.as_char_slice().last() == Some(&'/') {
-        path.pop();
+        path = path.slice_to(path.char_count() - 1);
     }
 
     // 4: No slashes left => return period.
-    let Some(last_slash) = path.chars().rev().position(|c| c == '/') else {
-        return "."L.to_owned()
+    let Some(last_slash) = path.chars().rposition(|c| c == '/') else {
+        return L!(".");
     };
 
     // 5: Remove trailing non-slashes.
+    path = path.slice_to(last_slash + 1);
+
     // 6: Skip as permitted.
     // 7: Remove trailing slashes again.
-    path = path
-        .chars()
-        .rev()
-        .skip(last_slash + 1)
-        .skip_while(|&c| c == '/')
-        .collect::<WString>()
-        .chars()
-        .rev()
-        .collect();
+    while path.as_char_slice().last() == Some(&'/') {
+        path = path.slice_to(path.char_count() - 1);
+    }
 
     // 8: Empty => return slash.
     if path.is_empty() {
-        path = "/"L.to_owned();
+        return L!("/");
     }
     path
 }
 
 /// Wide character version of basename().
-#[widestrs]
-pub fn wbasename(path: impl AsRef<wstr>) -> WString {
-    let path = path.as_ref();
+pub fn wbasename(mut path: &wstr) -> &wstr {
     // This follows OpenGroup basename recipe.
     // 1: empty => allowed to return ".". This is what system impls do.
     if path.is_empty() {
-        return "."L.to_owned();
+        return L!(".");
     }
 
     // 2: Skip as permitted.
     // 3: All slashes => return slash.
-    if !path.is_empty() && path.chars().find(|&c| c != '/').is_none() {
-        return "/"L.to_owned();
+    if !path.is_empty() && path.chars().all(|c| c == '/') {
+        return L!("/");
     }
 
     // 4: Remove trailing slashes.
+    while path.as_char_slice().last() == Some(&'/') {
+        path = path.slice_to(path.char_count() - 1);
+    }
+
     // 5: Remove up to and including last slash.
-    path.chars()
-        .rev()
-        .skip_while(|&c| c == '/')
-        .take_while(|&c| c != '/')
-        .collect::<WString>()
-        .chars()
-        .rev()
-        .collect()
+    if let Some(last_slash) = path.chars().rposition(|c| c == '/') {
+        path = path.slice_from(last_slash + 1);
+    }
+    path
 }
 
 /// Wide character version of mkdir.

fish-rust/src/wutil/tests.rs

@@ -1,60 +1,53 @@
 use super::*;
-use libc::PATH_MAX;
+use libc;
+use widestring_suffix::widestrs;
 
-macro_rules! test_cases_wdirname_wbasename {
-    ($($name:ident: $test:expr),*  $(,)?) => {
-        $(
-            #[test]
-            fn $name() {
-                let (path, dir, base) = $test;
-                let actual = wdirname(WString::from(path));
-                assert_eq!(actual, WString::from(dir), "Wrong dirname for {:?}", path);
-                let actual = wbasename(WString::from(path));
-                assert_eq!(actual, WString::from(base), "Wrong basename for {:?}", path);
-            }
-        )*
-    };
-}
+#[test]
+#[widestrs]
+fn test_wdirname_wbasename() {
+    // path, dir, base
+    struct Test(&'static wstr, &'static wstr, &'static wstr);
+    const testcases: &[Test] = &[
+        Test(""L, "."L, "."L),
+        Test("foo//"L, "."L, "foo"L),
+        Test("foo//////"L, "."L, "foo"L),
+        Test("/////foo"L, "/"L, "foo"L),
+        Test("//foo/////bar"L, "//foo"L, "bar"L),
+        Test("foo/////bar"L, "foo"L, "bar"L),
+        // Examples given in XPG4.2.
+        Test("/usr/lib"L, "/usr"L, "lib"L),
+        Test("usr"L, "."L, "usr"L),
+        Test("/"L, "/"L, "/"L),
+        Test("."L, "."L, "."L),
+        Test(".."L, "."L, ".."L),
+    ];
 
-/// Helper to return a string whose length greatly exceeds PATH_MAX.
-fn overlong_path() -> WString {
-    let mut longpath = WString::with_capacity((PATH_MAX * 2 + 10) as usize);
-    while longpath.len() < (PATH_MAX * 2) as usize {
+    for tc in testcases {
+        let Test(path, tc_dir, tc_base) = *tc;
+        let dir = wdirname(&path);
+        assert_eq!(
+            dir, tc_dir,
+            "\npath: {:?}, dir: {:?}, tc.dir: {:?}",
+            path, dir, tc_dir
+        );
+
+        let base = wbasename(&path);
+        assert_eq!(
+            base, tc_base,
+            "\npath: {:?}, base: {:?}, tc.base: {:?}",
+            path, base, tc_base
+        );
+    }
+
+    // Ensure strings which greatly exceed PATH_MAX still work (#7837).
+    const PATH_MAX: usize = libc::PATH_MAX as usize;
+    let mut longpath = WString::new();
+    longpath.reserve(PATH_MAX * 2 + 10);
+    while longpath.char_count() <= PATH_MAX * 2 {
         longpath.push_str("/overlong");
     }
-    return longpath;
-}
-
-test_cases_wdirname_wbasename! {
-    wdirname_wbasename_test_1: ("", ".", "."),
-    wdirname_wbasename_test_2: ("foo//", ".", "foo"),
-    wdirname_wbasename_test_3: ("foo//////", ".", "foo"),
-    wdirname_wbasename_test_4: ("/////foo", "/", "foo"),
-    wdirname_wbasename_test_5: ("/////foo", "/", "foo"),
-    wdirname_wbasename_test_6: ("//foo/////bar", "//foo", "bar"),
-    wdirname_wbasename_test_7: ("foo/////bar", "foo", "bar"),
-    // Examples given in XPG4.2.
-    wdirname_wbasename_test_8: ("/usr/lib", "/usr", "lib"),
-    wdirname_wbasename_test_9: ("usr", ".", "usr"),
-    wdirname_wbasename_test_10: ("/", "/", "/"),
-    wdirname_wbasename_test_11: (".", ".", "."),
-    wdirname_wbasename_test_12: ("..", ".", ".."),
-}
-
-// Ensures strings which greatly exceed PATH_MAX still work (#7837).
-#[test]
-fn test_overlong_wdirname_wbasename() {
-    let path = overlong_path();
-    let dir = {
-        let mut longpath_dir = path.clone();
-        let last_slash = longpath_dir.chars().rev().position(|c| c == '/').unwrap();
-        longpath_dir.truncate(longpath_dir.len() - last_slash - 1);
-        longpath_dir
-    };
-    let base = "overlong";
-
-    let actual = wdirname(&path);
-    assert_eq!(actual, dir, "Wrong dirname for {:?}", path);
-    let actual = wbasename(&path);
-    assert_eq!(actual, base, "Wrong basename for {:?}", path);
+    let last_slash = longpath.chars().rposition(|c| c == '/').unwrap();
+    let longpath_dir = &longpath[..last_slash];
+    assert_eq!(wdirname(&longpath), longpath_dir);
+    assert_eq!(wbasename(&longpath), "overlong"L);
 }