tidyup:minor api cleanup and explanation

Author: alexcu2718

README.md

@@ -122,7 +122,7 @@ To avoid issues, use --same-file-system when traversing symlinks. Both fd and fi
 
 - **getdirentries64: Optimised approach following a very similar approach to the above method**
 
-- **find_char_in_word**: Locates the first occurrence of a byte in a 64-bit word using SWAR (SIMD within a register), implemented as a const function
+- **find_char_in_word/find_last_char_in_word**: Locates the first/last occurrence of a byte in a 64-bit word using SWAR (SIMD within a register), implemented as a const function
 
 - **Compile-time colour mapping**: A compile-time perfect hashmap for colouring file paths, defined in a [separate repository](https://github.com/alexcu2718/compile_time_ls_colours)
 
@@ -141,29 +141,33 @@ Check source code for further explanation [in src/utils.rs](./src/utils.rs#L195)
 pub const unsafe fn dirent_const_time_strlen(drnt: *const dirent64) -> usize {
     use core::num::NonZeroU64;
     /*The only unsafe action is dereferencing the pointer
-     This MUST be validated beforehand */
-    const LO_U64:u64=u64::from_ne_bytes([0x01; size_of::<u64>()]); 
+    This MUST be validated beforehand */
+    const LO_U64: u64 = u64::from_ne_bytes([0x01; size_of::<u64>()]);
     const HI_U64: u64 = u64::from_ne_bytes([0x80; size_of::<u64>()]);
-     // Create a mask for the first 3 bytes in the case where reclen==24
+    // Create a mask for the first 3 bytes in the case where reclen==24
     const MASK: u64 = u64::from_ne_bytes([0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x00]);
-    const DIRENT_HEADER_START: usize = std::mem::offset_of!(dirent64, d_name);
-    let reclen = unsafe { (*drnt).d_reclen as usize }; 
+    const DIRENT_HEADER_START: usize = core::mem::offset_of!(dirent64, d_name);
+    let reclen = unsafe { (*drnt).d_reclen as usize };
     // Access the last 8 bytes of the word (this is an aligned read due to kernel providing 8 byte aligned dirent structs!)
     let last_word: u64 = unsafe { *(drnt.byte_add(reclen - 8).cast::<u64>()) };
     // reclen is always multiple of 8 so alignment is guaranteed
     let mask = MASK * ((reclen == 24) as u64); // branchless mask
     let candidate_pos = last_word | mask; //Mask out the false nulls when d_name is short 
     //The idea is to convert each 0-byte to 0x80, and each nonzero byte to 0x00
-    let zero_bit = unsafe { // Use specialised instructions (ctl_nonzero) 
+    let zero_bit = unsafe {
+        // Use specialised instructions (ctlz_nonzero)
         //to avoid 0 check for bitscan forward so it compiles to tzcnt on most CPU's
-            NonZeroU64::new_unchecked(candidate_pos.wrapping_sub(LO_U64) & !candidate_pos & HI_U64)
-        };
+        NonZeroU64::new_unchecked(candidate_pos.wrapping_sub(LO_U64) & !candidate_pos & HI_U64)
+    };
 
+    // Find the position of the null terminator
     #[cfg(target_endian = "little")]
-    let byte_pos = 8 - (zero_bit.trailing_zeros() >> 3) as usize;
+    let byte_pos = (zero_bit.trailing_zeros() >> 3) as usize;
     #[cfg(target_endian = "big")]
-    let byte_pos = 8 - (zero_bit.leading_zeros() >> 3) as usize;
-    reclen - DIRENT_HEADER_START - byte_pos
+    let byte_pos = (zero_bit.leading_zeros() >> 3) as usize;
+    // reclen-DIRENT_HEADER start is the maximum size of the string
+    // we then use the position of the `true` null terminator and subtract the 8, it's junk.
+    reclen - DIRENT_HEADER_START + byte_pos - 8
 }
 
 

benches/dirent_bench.rs

@@ -6,41 +6,33 @@ use std::hint::black_box;
 #[inline]
 //modified version to work for this test function(copy pasted really)
 pub const unsafe fn dirent_const_time_strlen(dirent: *const LibcDirent64) -> usize {
-    // Offset from the start of the struct to the beginning of d_name.
     const DIRENT_HEADER_START: usize = core::mem::offset_of!(LibcDirent64, d_name);
-    // Access the last field and then round up to find the minimum struct size
     const MINIMUM_DIRENT_SIZE: usize = DIRENT_HEADER_START.next_multiple_of(8);
-
     const LO_U64: u64 = u64::from_ne_bytes([0x01; size_of::<u64>()]);
     const HI_U64: u64 = u64::from_ne_bytes([0x80; size_of::<u64>()]);
-
     let reclen = unsafe { (*dirent).d_reclen } as usize;
-
     /*
       Read the last 8 bytes of the struct as a u64.
     This works because dirents are always 8-byte aligned. */
     // SAFETY: We're indexing in bounds within the pointer (it is guaranteed aligned by the kernel)
     let last_word: u64 = unsafe { *(dirent.byte_add(reclen - 8).cast::<u64>()) };
- 
 
     const MASK: u64 = u64::from_ne_bytes([0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x00]);
 
     let mask: u64 = MASK * ((reclen == MINIMUM_DIRENT_SIZE) as u64);
-   
+
     let candidate_pos: u64 = last_word | mask;
 
     let zero_bit = unsafe {
         NonZeroU64::new_unchecked(candidate_pos.wrapping_sub(LO_U64) & !candidate_pos & HI_U64)
     };
+    // Find the position of the null terminator
     #[cfg(target_endian = "little")]
-    let byte_pos = 8 - (zero_bit.trailing_zeros() >> 3) as usize;
+    let byte_pos = (zero_bit.trailing_zeros() >> 3) as usize;
     #[cfg(target_endian = "big")]
-    let byte_pos = 8 - (zero_bit.leading_zeros() >> 3) as usize;
+    let byte_pos = (zero_bit.leading_zeros() >> 3) as usize;
 
-    /*  Final length:
-    total record length - header size - null byte position
-    */
-    reclen - DIRENT_HEADER_START - byte_pos
+    reclen - DIRENT_HEADER_START + byte_pos - 8
 }
 
 #[repr(C)]

src/config.rs

@@ -3,6 +3,7 @@ use crate::cli_helpers::{SizeFilter, TimeFilter};
 use crate::glob_to_regex;
 use crate::{DirEntry, FileType, SearchConfigError};
 use core::num::NonZeroU32;
+use core::ops::Deref;
 use core::time::Duration;
 use regex::bytes::{Regex, RegexBuilder};
 use std::time::UNIX_EPOCH;
@@ -76,45 +77,47 @@ impl FileTypeFilter {
         }
     }
 
-    /// Parses a character into a `FileTypeFilter`
-    ///
-    /// This method converts a single character into the corresponding file type filter,
-    /// which is useful for parsing command-line arguments or configuration files.
-    ///
-    /// # Parameters
-    /// - `c`: The character to parse into a file type filter
-    ///
-    /// # Returns
-    /// - `Ok(FileTypeFilter)` if the character represents a valid file type
-    /// - `Err(String)` with an error message if the character is invalid
-    ///
-    /// # Supported Characters
-    /// - `'d'` - Directory
-    /// - `'u'` - Unknown file type  
-    /// - `'l'` - Symbolic link
-    /// - `'f'` - Regular file
-    /// - `'p'` - Named pipe (FIFO)
-    /// - `'c'` - Character device
-    /// - `'b'` - Block device
-    /// - `'s'` - Socket
-    /// - `'e'` - Empty file
-    /// - `'x'` - Executable file
-    ///
-    /// # Examples
-    /// ```
-    /// # use fdf::FileTypeFilter;
-    /// assert!(FileTypeFilter::from_char('d').is_ok());
-    /// assert!(FileTypeFilter::from_char('f').is_ok());
-    /// assert!(FileTypeFilter::from_char('z').is_err()); // Invalid character
-    ///
-    /// let filter = FileTypeFilter::from_char('l').unwrap();
-    /// assert!(matches!(filter, FileTypeFilter::Symlink));
-    /// ```
-    ///
-    /// # Errors
-    /// Returns an error if the character does not correspond to any known file type.
-    /// The error message includes the invalid character and suggests using `--help`
-    /// to see valid types.
+    /**
+     Parses a character into a `FileTypeFilter`
+
+     This method converts a single character into the corresponding file type filter,
+     which is useful for parsing command-line arguments or configuration files.
+
+     # Parameters
+     - `c`: The character to parse into a file type filter
+
+     # Returns
+     - `Ok(FileTypeFilter)` if the character represents a valid file type
+     - `Err(String)` with an error message if the character is invalid
+
+     # Supported Characters
+     - `'d'` - Directory
+     - `'u'` - Unknown file type
+     - `'l'` - Symbolic link
+     - `'f'` - Regular file
+     - `'p'` - Named pipe (FIFO)
+     - `'c'` - Character device
+     - `'b'` - Block device
+     - `'s'` - Socket
+     - `'e'` - Empty file
+     - `'x'` - Executable file
+
+     # Examples
+     ```
+     # use fdf::FileTypeFilter;
+     assert!(FileTypeFilter::from_char('d').is_ok());
+     assert!(FileTypeFilter::from_char('f').is_ok());
+     assert!(FileTypeFilter::from_char('z').is_err()); // Invalid character
+
+     let filter = FileTypeFilter::from_char('l').unwrap();
+     assert!(matches!(filter, FileTypeFilter::Symlink));
+     ```
+
+     # Errors
+     Returns an error if the character does not correspond to any known file type.
+     The error message includes the invalid character and suggests using `--help`
+     to see valid types.
+    */
     pub fn from_char(c: char) -> core::result::Result<Self, String> {
         match c {
             'd' => Ok(Self::Directory),
@@ -249,7 +252,7 @@ impl SearchConfig {
     /// Checks for extension match via memchr
     pub fn matches_extension<S>(&self, entry: &S) -> bool
     where
-        S: core::ops::Deref<Target = [u8]>,
+        S: Deref<Target = [u8]>,
     {
         debug_assert!(
             !entry.contains(&b'/'),
@@ -267,10 +270,12 @@ impl SearchConfig {
         reason = "Only checking regular files"
     )]
     #[allow(clippy::cast_sign_loss)] //signloss dont matter here
-    /// Applies the configured size filter to a directory entry, if any.
-    /// For regular files the size is checked directly.
-    /// For symlinks, the target is resolved first and then checked if it is a regular file.
-    /// Other file types are ignored.
+    /**
+     Applies the configured size filter to a directory entry, if any.
+     For regular files the size is checked directly.
+     For symlinks, the target is resolved first and then checked if it is a regular file.
+     Other file types are ignored.
+    */
     pub fn matches_size(&self, entry: &DirEntry) -> bool {
         let Some(filter_size) = self.size_filter else {
             return true; // No filter means always match
@@ -281,17 +286,10 @@ impl SearchConfig {
                 .file_size()
                 .ok()
                 .is_some_and(|sz| filter_size.is_within_size(sz as _)),
-
-            FileType::Symlink => {
-                if let Ok(path) = entry.to_full_path() {
-                    if path.is_regular_file() {
-                        if let Ok(sz) = entry.file_size() {
-                            return filter_size.is_within_size(sz as _);
-                        }
-                    }
-                }
-                false
-            }
+            // Resolve to full path first, this basically avoids broken symlinks
+            FileType::Symlink => entry.to_full_path_with_stat().is_ok_and(|(path, statted)| {
+                path.is_regular_file() && filter_size.is_within_size(statted.st_size as _)
+            }),
 
             _ => false,
         }
@@ -344,8 +342,6 @@ impl SearchConfig {
     #[expect(clippy::indexing_slicing, reason = "used for debug assert")]
     /// Checks if the path or file name matches the regex filter
     /// If `full_path` is false, only checks the filename
-    ///
-    /// Use a branchless trick to do indexing
     pub fn matches_path(&self, dir: &DirEntry, full_path: bool) -> bool {
         debug_assert!(
             !dir.file_name().contains(&b'/'),

src/direntry.rs

@@ -781,6 +781,35 @@ impl DirEntry {
             })
         })
     }
+    #[inline]
+    /// Similar to above function but modified for internal use within size filtering.
+    /// TODO, make this public at some point.
+    /// This just avoids calling stat twice basically.
+    pub(crate) fn to_full_path_with_stat(&self) -> Result<(Self, stat)> {
+        debug_assert!(
+            self.is_symlink(),
+            "in this private function we expect the file type to always be symlinks!"
+        );
+        self.get_realpath(|full_path| {
+            let file_name_index = full_path.to_bytes().file_name_index();
+
+            let statted = self.get_stat()?;
+
+            let (file_type, ino) = (FileType::from_stat(&statted), access_stat!(statted, st_ino));
+
+            Ok((
+                Self {
+                    path: full_path.into(),
+                    file_type,
+                    inode: ino,
+                    depth: self.depth,
+                    file_name_index,
+                    is_traversible_cache: Cell::new(Some(file_type == FileType::Directory)),
+                },
+                statted,
+            ))
+        })
+    }
 
     #[inline]
     /**

src/lib.rs

@@ -177,7 +177,9 @@ mod test;
 pub use buffer::{AlignedBuffer, ValueType};
 
 mod memchr_derivations;
-pub use memchr_derivations::{contains_zero_byte, find_char_in_word, find_zero_byte_u64, memrchr};
+pub use memchr_derivations::{
+    contains_zero_byte, find_char_in_word, find_last_char_in_word, find_zero_byte_u64, memrchr,
+};
 mod direntry;
 pub use direntry::DirEntry;
 
@@ -398,6 +400,8 @@ impl Finder {
                 || {
                     // Fast path: only calls stat IFF self.starting_filesystem is Some
                     debug_assert!(!self.search_config.follow_symlinks,"we expect follow symlinks to be disabled when following this path");
+
+
                     self.starting_filesystem.is_none_or(|start_dev| {
                         dir.get_stat()
                             .is_ok_and(|statted| start_dev == access_stat!(statted, st_dev))
@@ -415,11 +419,12 @@ impl Finder {
         }
 
         // Symlinks that may point to directories
-        // This could be optimised a bit, symlinks are a beast due to their complexity.
         // self.search_config.follow_symlinks <=> inode_cache is some
         FileType::Symlink
             if self.inode_cache.as_ref().is_some_and(|cache| {
                 debug_assert!(self.search_config.follow_symlinks,"we expect follow symlinks to be enabled when following this path");
+
+
                 dir.get_stat().is_ok_and(|stat| {
                     FileType::from_stat(&stat) == FileType::Directory &&
                     // Check filesystem boundary