[pbs-devel] [PATCH pathpatterns v4] match_list: updated `matches()`, to only retrieve file mode if necessary

[Gabriel Goller <g.goller@proxmox.com>]

Updated `matches()` function, which now takes the `GetFileMode` trait and returns a
Result. `GetFileMode` is a function that should return the `file_mode`. `matches()`
will go through the patterns and match by path only until it finds a pattern which
does not have `MatchFlag::ANY_FILE_TYPE`, in case it will call the
`get_file_mode`, which will return a `file_mode`. This will ensure that the
`get()` (which in our case executes `stat()`) will only be run once(at most),
every pattern will only be processed once and that the order of the patterns
will be respected. `GetFileMode` is also implemented for `Option<u32>` (so that
we can simply pass options of the `file_mode`) and `u32` (returning
`std::convert::Infallible`, so that we can pass a u32 mode directly).

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---

changes v3:
 - changed `matches()` and `matches_exact()` to retrieve the file_mode lazily.
   Allowed these functions to take in a trait `GetFileMode`, so that we can pass
   closures and options that contain the `file_mode`.

changes v2:
 - updated the `matches_path()` function to take a closure and renamed
   it to `matches_mode_lazy()`. This allows us to only match once, so we
   don't need to match before and after `stat()` (without and with
   `file_mode`) anymore.

changes v1:
 - added `matches_path()` function, which matches by path only and returns an
   error if the `file_mode` is required.


 src/lib.rs        |  72 +++++-----
 src/match_list.rs | 335 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 315 insertions(+), 92 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index cb89917..998cdae 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -31,17 +31,17 @@
 //!     MatchEntry::include(Pattern::path("bananas/curved.*")?),
 //! ];
 //!
-//! assert_eq!(list.matches("/things", None), None);
-//! assert_eq!(list.matches("/things/shop", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/bananas", None), Some(MatchType::Exclude));
-//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Some(MatchType::Include));
+//! assert_eq!(list.matches("/things", None), Ok(None));
+//! assert_eq!(list.matches("/things/shop", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/bananas", None), Ok(Some(MatchType::Exclude)));
+//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
 //!
 //! // this will exclude the curved.bak file
 //! list.push(MatchEntry::exclude(Pattern::path("curved.bak")?));
-//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Some(MatchType::Exclude));
+//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Ok(Some(MatchType::Exclude)));
 //! list.pop();
-//! # assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Some(MatchType::Include));
+//! # assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
 //!
 //! // but this will not:
 //! list.push(
@@ -49,40 +49,40 @@
 //!         .flags(MatchFlag::ANCHORED)
 //! );
 //! // or: list.push
-//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Some(MatchType::Include));
+//! assert_eq!(list.matches("/things/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
 //! list.pop();
 //!
 //! // let's check some patterns, anything starting with a 'c', 'f' or 's':
 //! let mut list = vec![MatchEntry::include(Pattern::path("[cfs]*")?)];
-//! assert_eq!(list.matches("/things", None), None);
-//! assert_eq!(list.matches("/things/file1.dat", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/file2.dat", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/info.txt", None), None);
-//! assert_eq!(list.matches("/things/shop/apples", None), None);
-//! assert_eq!(list.matches("/things/shop/apples/gala.txt", None), None);
-//! assert_eq!(list.matches("/things/shop/apples/golden-delicious.txt", None), None);
-//! assert_eq!(list.matches("/things/shop/bananas", None), None);
-//! assert_eq!(list.matches("/things/shop/bananas/straight.txt", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/shop/bananas/curved.bak", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/bananas/other.txt", None), None);
+//! assert_eq!(list.matches("/things", None), Ok(None));
+//! assert_eq!(list.matches("/things/file1.dat", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/file2.dat", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/info.txt", None), Ok(None));
+//! assert_eq!(list.matches("/things/shop/apples", None), Ok(None));
+//! assert_eq!(list.matches("/things/shop/apples/gala.txt", None), Ok(None));
+//! assert_eq!(list.matches("/things/shop/apples/golden-delicious.txt", None), Ok(None));
+//! assert_eq!(list.matches("/things/shop/bananas", None), Ok(None));
+//! assert_eq!(list.matches("/things/shop/bananas/straight.txt", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/bananas/other.txt", None), Ok(None));
 //!
 //! // If we add `**` we end up including the entire `shop/` subtree:
 //! list.push(MatchEntry::include(Pattern::path("[cfs]*/**")?));
-//! assert_eq!(list.matches("/things", None), None);
-//! assert_eq!(list.matches("/things/file1.dat", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/file2.dat", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/info.txt", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/apples", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/apples/gala.txt", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/shop/apples/golden-delicious.txt", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/bananas", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/bananas/straight.txt", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/shop/bananas/curved.bak", None), Some(MatchType::Include));
-//! assert_eq!(list.matches("/shop/bananas/other.txt", None), Some(MatchType::Include));
+//! assert_eq!(list.matches("/things", None), Ok(None));
+//! assert_eq!(list.matches("/things/file1.dat", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/file2.dat", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/info.txt", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/apples", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/apples/gala.txt", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/shop/apples/golden-delicious.txt", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/bananas", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/bananas/straight.txt", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/things/shop/bananas/curved.txt", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/shop/bananas/curved.bak", None), Ok(Some(MatchType::Include)));
+//! assert_eq!(list.matches("/shop/bananas/other.txt", None), Ok(Some(MatchType::Include)));
 //! # Ok(())
 //! # }
 //! # test().unwrap()
@@ -92,7 +92,9 @@ mod match_list;
 mod pattern;
 
 #[doc(inline)]
-pub use match_list::{MatchEntry, MatchFlag, MatchList, MatchPattern, MatchType};
+pub use match_list::{
+    FileModeRequired, GetFileMode, MatchEntry, MatchFlag, MatchList, MatchPattern, MatchType,
+};
 
 #[doc(inline)]
 pub use pattern::{ParseError, Pattern, PatternFlag};
diff --git a/src/match_list.rs b/src/match_list.rs
index c5b14e0..69b1fdb 100644
--- a/src/match_list.rs
+++ b/src/match_list.rs
@@ -1,5 +1,4 @@
 //! Helpers for include/exclude lists.
-
 use bitflags::bitflags;
 
 use crate::PatternFlag;
@@ -39,6 +38,46 @@ impl Default for MatchFlag {
     }
 }
 
+#[derive(Debug, PartialEq)]
+pub struct FileModeRequired;
+
+impl std::fmt::Display for FileModeRequired {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "File mode is required for matching")
+    }
+}
+
+impl std::error::Error for FileModeRequired {}
+
+pub trait GetFileMode {
+    type Error;
+    fn get(self) -> Result<u32, Self::Error>;
+}
+
+impl<T, E> GetFileMode for T
+where
+    T: FnOnce() -> Result<u32, E>,
+{
+    type Error = E;
+    fn get(self) -> Result<u32, Self::Error> {
+        self()
+    }
+}
+
+impl GetFileMode for Option<u32> {
+    type Error = FileModeRequired;
+    fn get(self) -> Result<u32, Self::Error> {
+        self.ok_or(FileModeRequired {})
+    }
+}
+
+impl GetFileMode for u32 {
+    type Error = std::convert::Infallible;
+    fn get(self) -> Result<u32, Self::Error> {
+        Ok(self)
+    }
+}
+
 /// A pattern entry. (Glob patterns or literal patterns.)
 // Note:
 // For regex we'd likely use the POSIX extended REs via `regexec(3)`, since we're targetting
@@ -211,7 +250,7 @@ impl MatchEntry {
     pub fn matches_mode(&self, file_mode: u32) -> bool {
         // bitflags' `.contains` means ALL bits must be set, if they are all set we don't
         // need to check the mode...
-        if self.flags.contains(MatchFlag::ANY_FILE_TYPE) {
+        if !self.needs_file_mode() {
             return true;
         }
 
@@ -304,12 +343,21 @@ impl MatchEntry {
 
         self.matches_path_exact(path)
     }
+
+    pub fn needs_file_mode(&self) -> bool {
+        let flags = (self.flags & MatchFlag::ANY_FILE_TYPE).bits();
+        if flags != 0 && flags != MatchFlag::ANY_FILE_TYPE.bits() {
+            return true;
+        }
+        false
+    }
 }
 
 #[doc(hidden)]
 pub trait MatchListEntry {
     fn entry_matches(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType>;
     fn entry_matches_exact(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType>;
+    fn entry_needs_file_mode(&self) -> bool;
 }
 
 impl MatchListEntry for &'_ MatchEntry {
@@ -328,6 +376,10 @@ impl MatchListEntry for &'_ MatchEntry {
             None
         }
     }
+
+    fn entry_needs_file_mode(&self) -> bool {
+        self.needs_file_mode()
+    }
 }
 
 impl MatchListEntry for &'_ &'_ MatchEntry {
@@ -346,6 +398,10 @@ impl MatchListEntry for &'_ &'_ MatchEntry {
             None
         }
     }
+
+    fn entry_needs_file_mode(&self) -> bool {
+        self.needs_file_mode()
+    }
 }
 
 /// This provides [`matches`](MatchList::matches) and [`matches_exact`](MatchList::matches_exact)
@@ -361,19 +417,18 @@ impl MatchListEntry for &'_ &'_ MatchEntry {
 /// This makes it easier to use slices over entries or references to entries.
 pub trait MatchList {
     /// Check whether this list contains anything matching a prefix of the specified path, and the
-    /// specified file mode.
-    fn matches<T: AsRef<[u8]>>(&self, path: T, file_mode: Option<u32>) -> Option<MatchType> {
-        self.matches_do(path.as_ref(), file_mode)
-    }
-
-    fn matches_do(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType>;
-
-    /// Check whether this list contains anything exactly matching the path and mode.
-    fn matches_exact<T: AsRef<[u8]>>(&self, path: T, file_mode: Option<u32>) -> Option<MatchType> {
-        self.matches_exact_do(path.as_ref(), file_mode)
-    }
-
-    fn matches_exact_do(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType>;
+    /// specified file mode. Gets the file_mode lazily, only if needed.
+    fn matches<T, U>(&self, path: T, get_file_mode: U) -> Result<Option<MatchType>, U::Error>
+    where
+        T: AsRef<[u8]>,
+        U: GetFileMode;
+
+    /// Check whether this list contains anything exactly matching the path and mode. Gets the
+    /// file_mode lazily, only if needed.
+    fn matches_exact<T, U>(&self, path: T, get_file_mode: U) -> Result<Option<MatchType>, U::Error>
+    where
+        T: AsRef<[u8]>,
+        U: GetFileMode;
 }
 
 impl<'a, T> MatchList for T
@@ -383,30 +438,48 @@ where
     <&'a T as IntoIterator>::IntoIter: DoubleEndedIterator,
     <&'a T as IntoIterator>::Item: MatchListEntry,
 {
-    fn matches_do(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType> {
+    fn matches<U, G>(&self, path: U, get_file_mode: G) -> Result<Option<MatchType>, G::Error>
+    where
+        U: AsRef<[u8]>,
+        G: GetFileMode,
+    {
         // This is an &self method on a `T where T: 'a`.
         let this: &'a Self = unsafe { std::mem::transmute(self) };
 
+        let mut get_file_mode = Some(get_file_mode);
+        let mut file_mode = None;
+
         for m in this.into_iter().rev() {
-            if let Some(mt) = m.entry_matches(path, file_mode) {
-                return Some(mt);
+            if file_mode.is_none() && m.entry_needs_file_mode() {
+                file_mode = Some(get_file_mode.take().unwrap().get()?);
+            }
+            if let Some(mt) = m.entry_matches(path.as_ref(), file_mode) {
+                return Ok(Some(mt));
             }
         }
-
-        None
+        Ok(None)
     }
 
-    fn matches_exact_do(&self, path: &[u8], file_mode: Option<u32>) -> Option<MatchType> {
+    fn matches_exact<U, G>(&self, path: U, get_file_mode: G) -> Result<Option<MatchType>, G::Error>
+    where
+        U: AsRef<[u8]>,
+        G: GetFileMode,
+    {
         // This is an &self method on a `T where T: 'a`.
         let this: &'a Self = unsafe { std::mem::transmute(self) };
 
+        let mut get_file_mode = Some(get_file_mode);
+        let mut file_mode = None;
+
         for m in this.into_iter().rev() {
-            if let Some(mt) = m.entry_matches_exact(path, file_mode) {
-                return Some(mt);
+            if file_mode.is_none() && m.entry_needs_file_mode() {
+                file_mode = Some(get_file_mode.take().unwrap().get()?);
+            }
+            if let Some(mt) = m.entry_matches_exact(path.as_ref(), file_mode) {
+                return Ok(Some(mt));
             }
         }
-
-        None
+        Ok(None)
     }
 }
 
@@ -415,20 +488,20 @@ fn assert_containers_implement_match_list() {
     use std::iter::FromIterator;
 
     let vec = vec![MatchEntry::include(crate::Pattern::path("a*").unwrap())];
-    assert_eq!(vec.matches("asdf", None), Some(MatchType::Include));
+    assert_eq!(vec.matches("asdf", None), Ok(Some(MatchType::Include)));
 
     // FIXME: ideally we can make this work as well!
     let vd = std::collections::VecDeque::<MatchEntry>::from_iter(vec.clone());
-    assert_eq!(vd.matches("asdf", None), Some(MatchType::Include));
+    assert_eq!(vd.matches("asdf", None), Ok(Some(MatchType::Include)));
 
     let list: &[MatchEntry] = &vec[..];
-    assert_eq!(list.matches("asdf", None), Some(MatchType::Include));
+    assert_eq!(list.matches("asdf", None), Ok(Some(MatchType::Include)));
 
     let list: Vec<&MatchEntry> = vec.iter().collect();
-    assert_eq!(list.matches("asdf", None), Some(MatchType::Include));
+    assert_eq!(list.matches("asdf", None), Ok(Some(MatchType::Include)));
 
     let list: &[&MatchEntry] = &list[..];
-    assert_eq!(list.matches("asdf", None), Some(MatchType::Include));
+    assert_eq!(list.matches("asdf", None), Ok(Some(MatchType::Include)));
 }
 
 #[test]
@@ -443,25 +516,28 @@ fn test_file_type_matches() {
     ];
     assert_eq!(
         matchlist.matches("a_dir", Some(libc::S_IFDIR)),
-        Some(MatchType::Include)
+        Ok(Some(MatchType::Include))
     );
     assert_eq!(
         matchlist.matches("/a_dir", Some(libc::S_IFDIR)),
-        Some(MatchType::Include)
+        Ok(Some(MatchType::Include))
     );
-    assert_eq!(matchlist.matches("/a_dir", Some(libc::S_IFREG)), None);
+    assert_eq!(matchlist.matches("/a_dir", Some(libc::S_IFREG)), Ok(None));
 
     assert_eq!(
         matchlist.matches("/a_file", Some(libc::S_IFREG)),
-        Some(MatchType::Exclude)
+        Ok(Some(MatchType::Exclude))
     );
-    assert_eq!(matchlist.matches("/a_file", Some(libc::S_IFDIR)), None);
+    assert_eq!(matchlist.matches("/a_file", Some(libc::S_IFDIR)), Ok(None));
 
     assert_eq!(
         matchlist.matches("/another_dir", Some(libc::S_IFDIR)),
-        Some(MatchType::Exclude)
+        Ok(Some(MatchType::Exclude))
+    );
+    assert_eq!(
+        matchlist.matches("/another_dir", Some(libc::S_IFREG)),
+        Ok(None)
     );
-    assert_eq!(matchlist.matches("/another_dir", Some(libc::S_IFREG)), None);
 }
 
 #[test]
@@ -471,22 +547,25 @@ fn test_anchored_matches() {
     let matchlist = vec![
         MatchEntry::new(Pattern::path("file-a").unwrap(), MatchType::Include),
         MatchEntry::new(Pattern::path("some/path").unwrap(), MatchType::Include)
-            .flags(MatchFlag::ANCHORED),
+            .add_flags(MatchFlag::ANCHORED),
     ];
 
-    assert_eq!(matchlist.matches("file-a", None), Some(MatchType::Include));
+    assert_eq!(
+        matchlist.matches("file-a", None),
+        Ok(Some(MatchType::Include))
+    );
     assert_eq!(
         matchlist.matches("another/file-a", None),
-        Some(MatchType::Include)
+        Ok(Some(MatchType::Include))
     );
 
-    assert_eq!(matchlist.matches("some", None), None);
-    assert_eq!(matchlist.matches("path", None), None);
+    assert_eq!(matchlist.matches("some", None), Ok(None));
+    assert_eq!(matchlist.matches("path", None), Ok(None));
     assert_eq!(
         matchlist.matches("some/path", None),
-        Some(MatchType::Include)
+        Ok(Some(MatchType::Include))
     );
-    assert_eq!(matchlist.matches("another/some/path", None), None);
+    assert_eq!(matchlist.matches("another/some/path", None), Ok(None));
 }
 
 #[test]
@@ -495,7 +574,10 @@ fn test_literal_matches() {
         MatchPattern::Literal(b"/bin/mv".to_vec()),
         MatchType::Include,
     )];
-    assert_eq!(matchlist.matches("/bin/mv", None), Some(MatchType::Include));
+    assert_eq!(
+        matchlist.matches("/bin/mv", None),
+        Ok(Some(MatchType::Include))
+    );
 }
 
 #[test]
@@ -504,29 +586,168 @@ fn test_path_relativity() {
     let matchlist = vec![
         MatchEntry::new(Pattern::path("noslash").unwrap(), MatchType::Include),
         MatchEntry::new(Pattern::path("noslash-a").unwrap(), MatchType::Include)
-            .flags(MatchFlag::ANCHORED),
+            .add_flags(MatchFlag::ANCHORED),
         MatchEntry::new(Pattern::path("/slash").unwrap(), MatchType::Include),
         MatchEntry::new(Pattern::path("/slash-a").unwrap(), MatchType::Include)
-            .flags(MatchFlag::ANCHORED),
+            .add_flags(MatchFlag::ANCHORED),
     ];
-    assert_eq!(matchlist.matches("noslash", None), Some(MatchType::Include));
+    assert_eq!(
+        matchlist.matches("noslash", None),
+        Ok(Some(MatchType::Include))
+    );
     assert_eq!(
         matchlist.matches("noslash-a", None),
-        Some(MatchType::Include)
+        Ok(Some(MatchType::Include))
     );
-    assert_eq!(matchlist.matches("slash", None), None);
-    assert_eq!(matchlist.matches("/slash", None), Some(MatchType::Include));
-    assert_eq!(matchlist.matches("slash-a", None), None);
+    assert_eq!(matchlist.matches("slash", None), Ok(None));
+    assert_eq!(
+        matchlist.matches("/slash", None),
+        Ok(Some(MatchType::Include))
+    );
+    assert_eq!(matchlist.matches("slash-a", None), Ok(None));
     assert_eq!(
         matchlist.matches("/slash-a", None),
-        Some(MatchType::Include)
+        Ok(Some(MatchType::Include))
     );
 
     assert_eq!(
         matchlist.matches("foo/noslash", None),
-        Some(MatchType::Include)
+        Ok(Some(MatchType::Include))
+    );
+    assert_eq!(matchlist.matches("foo/noslash-a", None), Ok(None));
+    assert_eq!(matchlist.matches("foo/slash", None), Ok(None));
+    assert_eq!(matchlist.matches("foo/slash-a", None), Ok(None));
+}
+
+#[test]
+fn matches_path() {
+    use crate::Pattern;
+
+    let matchlist = vec![
+        MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude),
+        MatchEntry::new(Pattern::path("b*").unwrap(), MatchType::Exclude),
+    ];
+
+    assert_eq!(
+        matchlist.matches("ahsjdj", || Err(FileModeRequired {})),
+        Ok(Some(MatchType::Exclude))
+    );
+    let mut test = 1;
+    let result = matchlist.matches("bhshdf", || {
+        test += 1;
+        Ok::<u32, FileModeRequired>(libc::S_IFDIR)
+    });
+    assert_eq!(result, Ok(Some(MatchType::Exclude)));
+    assert_eq!(test, 1);
+
+    let matchlist = vec![
+        MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
+            .flags(MatchFlag::MATCH_DIRECTORIES),
+        MatchEntry::new(Pattern::path("b*").unwrap(), MatchType::Exclude),
+    ];
+
+    let mut test = 1;
+    let result = matchlist.matches("aa", || {
+        test += 1;
+        Ok::<u32, FileModeRequired>(libc::S_IFDIR)
+    });
+    assert_eq!(result, Ok(Some(MatchType::Exclude)));
+    assert_eq!(test, 2);
+    assert_eq!(
+        matchlist.matches("ahsjdj", || Err(FileModeRequired {})),
+        Err(FileModeRequired {})
+    );
+    assert_eq!(
+        matchlist.matches("bhshdf", || Err(FileModeRequired {})),
+        Ok(Some(MatchType::Exclude))
+    );
+    assert_eq!(
+        matchlist.matches("ahsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFREG)),
+        Ok(None)
+    );
+
+    let matchlist = vec![
+        MatchEntry::new(Pattern::path("b*").unwrap(), MatchType::Include),
+        MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
+            .flags(MatchFlag::MATCH_DIRECTORIES),
+    ];
+    assert_eq!(
+        matchlist.matches("ahsjdj", || Err(FileModeRequired {})),
+        Err(FileModeRequired {})
+    );
+    assert_eq!(
+        matchlist.matches("bhshdf", || Err(FileModeRequired {})),
+        Err(FileModeRequired {})
+    );
+    assert_eq!(
+        matchlist.matches("ahsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFDIR)),
+        Ok(Some(MatchType::Exclude))
+    );
+    assert_eq!(
+        matchlist.matches("ahsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFREG)),
+        Ok(None)
+    );
+    assert_eq!(
+        matchlist.matches("bbb", || Ok::<u32, FileModeRequired>(libc::S_IFREG)),
+        Ok(Some(MatchType::Include))
     );
-    assert_eq!(matchlist.matches("foo/noslash-a", None), None);
-    assert_eq!(matchlist.matches("foo/slash", None), None);
-    assert_eq!(matchlist.matches("foo/slash-a", None), None);
+
+    let matchlist = vec![
+        MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
+            .flags(MatchFlag::MATCH_DIRECTORIES),
+    ];
+
+    assert_eq!(
+        matchlist.matches("bbb", || Ok::<u32, FileModeRequired>(libc::S_IFDIR)),
+        Ok(None)
+    );
+
+    let matchlist = vec![
+        MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
+            .flags(MatchFlag::MATCH_DIRECTORIES),
+        MatchEntry::new(Pattern::path("b*").unwrap(), MatchType::Exclude)
+            .flags(MatchFlag::MATCH_REGULAR_FILES),
+    ];
+
+    assert_eq!(
+        matchlist.matches("ahsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFDIR)),
+        Ok(Some(MatchType::Exclude))
+    );
+    assert_eq!(
+        matchlist.matches("ahsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFREG)),
+        Ok(None)
+    );
+    assert_eq!(
+        matchlist.matches("bhsjdj", || Ok::<u32, FileModeRequired>(libc::S_IFREG)),
+        Ok(Some(MatchType::Exclude))
+    );
+    assert_eq!(
+        matchlist.matches("bhsjdj", libc::S_IFREG as u32),
+        Ok(Some(MatchType::Exclude))
+    );
+    assert_eq!(matchlist.matches("bhsjdj", 0), Ok(None));
+}
+
+#[test]
+fn match_entry_needs_flag() {
+    use crate::Pattern;
+    let match_entry =
+        MatchEntry::parse_pattern("a*/", PatternFlag::PATH_NAME, MatchType::Exclude).unwrap();
+    assert_eq!(match_entry.needs_file_mode(), true);
+
+    let match_entry = MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
+        .flags(MatchFlag::MATCH_REGULAR_FILES);
+    assert_eq!(match_entry.needs_file_mode(), true);
+
+    let match_entry = MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
+        .flags(MatchFlag::MATCH_DIRECTORIES | MatchFlag::MATCH_REGULAR_FILES);
+    assert_eq!(match_entry.needs_file_mode(), true);
+
+    let match_entry = MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
+        .flags(MatchFlag::ANCHORED | MatchFlag::MATCH_DIRECTORIES);
+    assert_eq!(match_entry.needs_file_mode(), true);
+
+    let match_entry = MatchEntry::new(Pattern::path("a*").unwrap(), MatchType::Exclude)
+        .flags(MatchFlag::ANY_FILE_TYPE);
+    assert_eq!(match_entry.needs_file_mode(), false);
 }
-- 
2.39.2