pxar: cleanup: refactor and rename exclude pattern

The original name PxarExcludePattern makes no sense anymore as the patterns are
also used to match filenames during restore of the archive.

Therefore, exclude_pattern.rs is moved to match_pattern.rs and PxarExcludePattern
rename to MatchPattern.
Further, since it makes more sense the MatchTypes are now declared as None,
Positive, Negative, PartialPositive or PartialNegative, as this makes more sense
and seems more readable.
Positive matches are those without '!' prefix, Negatives with '!' prefix.

This makes also the filename matching in the encoder/decoder more intuitive and
the logic was adapted accordingly.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>

src/bin/pxar.rs

@@ -66,7 +66,7 @@ fn extract_archive_from_reader<R: std::io::Read>(
     feature_flags: u64,
     allow_existing_dirs: bool,
     verbose: bool,
-    pattern: Option<Vec<pxar::PxarExcludePattern>>
+    pattern: Option<Vec<pxar::MatchPattern>>
 ) -> Result<(), Error> {
     let mut decoder = pxar::SequentialDecoder::new(reader, feature_flags, |path| {
         if verbose {
@@ -125,14 +125,14 @@ fn extract_archive(
     let mut pattern_list = Vec::new();
     if let Some(filename) = files_from {
         let dir = nix::dir::Dir::open("./", nix::fcntl::OFlag::O_RDONLY, nix::sys::stat::Mode::empty())?;
-        if let Some((mut pattern, _, _)) = pxar::PxarExcludePattern::from_file(dir.as_raw_fd(), filename)? {
+        if let Some((mut pattern, _, _)) = pxar::MatchPattern::from_file(dir.as_raw_fd(), filename)? {
             pattern_list.append(&mut pattern);
         }
     }
 
     for s in arg_pattern {
         let l = s.as_str().ok_or_else(|| format_err!("Invalid pattern string slice"))?;
-        let p = pxar::PxarExcludePattern::from_line(l.as_bytes())?
+        let p = pxar::MatchPattern::from_line(l.as_bytes())?
             .ok_or_else(|| format_err!("Invalid match pattern in arguments"))?;
         pattern_list.push(p);
     }

src/pxar.rs

@@ -65,8 +65,8 @@ pub use sequential_decoder::*;
 mod decoder;
 pub use decoder::*;
 
-mod exclude_pattern;
+mod match_pattern;
-pub use exclude_pattern::*;
+pub use match_pattern::*;
 
 mod dir_stack;
 pub use dir_stack::*;

src/pxar/encoder.rs

@@ -10,7 +10,7 @@ use super::flags;
 use super::format_definition::*;
 use super::binary_search_tree::*;
 use super::helper::*;
-use super::exclude_pattern::*;
+use super::match_pattern::*;
 use crate::tools::fs;
 use crate::tools::acl;
 use crate::tools::xattr;
@@ -131,7 +131,7 @@ impl <'a, W: Write> Encoder<'a, W> {
 
         let mut excludes = Vec::new();
         if skip_lost_and_found {
-            excludes.push(PxarExcludePattern::from_line(b"**/lost+found").unwrap().unwrap());
+            excludes.push(MatchPattern::from_line(b"**/lost+found").unwrap().unwrap());
         }
         me.encode_dir(dir, &stat, magic, excludes)?;
 
@@ -582,7 +582,13 @@ impl <'a, W: Write> Encoder<'a, W> {
         Ok(())
     }
 
-    fn encode_dir(&mut self, dir: &mut nix::dir::Dir, dir_stat: &FileStat, magic: i64, match_pattern: Vec<PxarExcludePattern>)  -> Result<(), Error> {
+    fn encode_dir(
+        &mut self,
+        dir: &mut nix::dir::Dir,
+        dir_stat: &FileStat,
+        magic: i64,
+        match_pattern: Vec<MatchPattern>,
+    ) -> Result<(), Error> {
 
         //println!("encode_dir: {:?} start {}", self.full_path(), self.writer_pos);
 
@@ -648,7 +654,7 @@ impl <'a, W: Write> Encoder<'a, W> {
 
         // Expand the exclude match pattern inherited from the parent by local entries, if present
         let mut local_match_pattern = match_pattern.clone();
-        let pxar_exclude = match PxarExcludePattern::from_file(rawfd, ".pxarexclude") {
+        let pxar_exclude = match MatchPattern::from_file(rawfd, ".pxarexclude") {
             Ok(Some((mut excludes, buffer, stat))) => {
                 local_match_pattern.append(&mut excludes);
                 Some((buffer, stat))
@@ -679,8 +685,8 @@ impl <'a, W: Write> Encoder<'a, W> {
                     Err(err) => bail!("fstat {:?} failed - {}", self.full_path(), err),
                 };
 
-                match match_exclude_pattern(&filename, &stat, &local_match_pattern) {
+                match match_filename(&filename, &stat, &local_match_pattern) {
-                    (MatchType::Exclude, _) => {
+                    (MatchType::Positive, _) => {
                         let filename_osstr = std::ffi::OsStr::from_bytes(filename.to_bytes());
                         eprintln!("matched by .pxarexclude entry - skipping: {:?}", self.full_path().join(filename_osstr));
                     },
@@ -1063,29 +1069,23 @@ impl <'a, W: Write> Encoder<'a, W> {
     }
 }
 
-// If there is a match, an updated PxarExcludePattern list to pass to the matched child is returned.
+// If there is a match, an updated MatchPattern list to pass to the matched child is returned.
-fn match_exclude_pattern(
+fn match_filename(
     filename: &CStr,
     stat: &FileStat,
-    match_pattern: &Vec<PxarExcludePattern>
+    match_pattern: &Vec<MatchPattern>
-) ->  (MatchType, Vec<PxarExcludePattern>) {
+) ->  (MatchType, Vec<MatchPattern>) {
     let mut child_pattern = Vec::new();
     let mut match_state = MatchType::None;
 
     for pattern in match_pattern {
         match pattern.matches_filename(filename, is_directory(&stat)) {
             MatchType::None =>  {},
-            MatchType::Exclude =>  match_state = MatchType::Exclude,
+            MatchType::Positive =>  match_state = MatchType::Positive,
-            MatchType::Include =>  match_state = MatchType::Include,
+            MatchType::Negative =>  match_state = MatchType::Negative,
-            MatchType::PartialExclude =>  {
+            match_type => {
-                if match_state != MatchType::Exclude && match_state != MatchType::Include {
+                if match_state != MatchType::Positive && match_state != MatchType::Negative {
-                    match_state = MatchType::PartialExclude;
+                    match_state = match_type;
-                }
-                child_pattern.push(pattern.get_rest_pattern());
-            },
-            MatchType::PartialInclude =>  {
-                if match_state != MatchType::Exclude && match_state != MatchType::Include {
-                    match_state = MatchType::PartialInclude;
                 }
                 child_pattern.push(pattern.get_rest_pattern());
             },

src/pxar/match_pattern.rs

@@ -5,12 +5,14 @@ use std::os::unix::io::{FromRawFd, RawFd};
 
 use failure::*;
 use libc::{c_char, c_int};
-use nix::fcntl::OFlag;
+use nix::fcntl;
+use nix::fcntl::{AtFlags, OFlag};
 use nix::errno::Errno;
 use nix::NixPath;
+use nix::sys::stat;
 use nix::sys::stat::{FileStat, Mode};
 
-pub const FNM_NOMATCH:  c_int = 1;
+pub const FNM_NOMATCH: c_int = 1;
 
 extern "C" {
     fn fnmatch(pattern: *const c_char, string: *const c_char, flags: c_int) -> c_int;
@@ -19,29 +21,33 @@ extern "C" {
 #[derive(Debug, PartialEq, Clone, Copy)]
 pub enum MatchType {
     None,
-    Exclude,
+    Positive,
-    Include,
+    Negative,
-    PartialExclude,
+    PartialPositive,
-    PartialInclude,
+    PartialNegative,
 }
 
 #[derive(Clone)]
-pub struct PxarExcludePattern {
+pub struct MatchPattern {
     pattern: CString,
-    match_exclude: bool,
+    match_positive: bool,
     match_dir_only: bool,
     split_pattern: (CString, CString),
 }
 
-impl PxarExcludePattern {
+impl MatchPattern {
-    pub fn from_file<P: ?Sized + NixPath>(parent_fd: RawFd, filename: &P) -> Result<Option<(Vec<PxarExcludePattern>, Vec<u8>, FileStat)>, Error> {
+    pub fn from_file<P: ?Sized + NixPath>(
-        let stat = match nix::sys::stat::fstatat(parent_fd, filename, nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW) {
+        parent_fd: RawFd,
+        filename: &P,
+    ) -> Result<Option<(Vec<MatchPattern>, Vec<u8>, FileStat)>, Error> {
+
+        let stat = match stat::fstatat(parent_fd, filename, AtFlags::AT_SYMLINK_NOFOLLOW) {
             Ok(stat) => stat,
             Err(nix::Error::Sys(Errno::ENOENT)) => return Ok(None),
             Err(err) => bail!("stat failed - {}", err),
         };
 
-        let filefd = nix::fcntl::openat(parent_fd, filename, OFlag::O_NOFOLLOW, Mode::empty())?;
+        let filefd = fcntl::openat(parent_fd, filename, OFlag::O_NOFOLLOW, Mode::empty())?;
         let mut file = unsafe {
             File::from_raw_fd(filefd)
         };
@@ -49,27 +55,27 @@ impl PxarExcludePattern {
         let mut content_buffer = Vec::new();
         let _bytes = file.read_to_end(&mut content_buffer)?;
 
-        let mut exclude_pattern = Vec::new();
+        let mut match_pattern = Vec::new();
         for line in content_buffer.split(|&c| c == b'\n') {
             if line.is_empty() {
                 continue;
             }
             if let Some(pattern) = Self::from_line(line)? {
-                exclude_pattern.push(pattern);
+                match_pattern.push(pattern);
             }
         }
 
-        Ok(Some((exclude_pattern, content_buffer, stat)))
+        Ok(Some((match_pattern, content_buffer, stat)))
     }
 
-    pub fn from_line(line: &[u8]) -> Result<Option<PxarExcludePattern>, Error> {
+    pub fn from_line(line: &[u8]) -> Result<Option<MatchPattern>, Error> {
         let mut input = line;
 
         if input.starts_with(b"#") {
             return Ok(None);
         }
 
-        let match_exclude = if input.starts_with(b"!") {
+        let match_positive = if input.starts_with(b"!") {
             // Reduce slice view to exclude "!"
             input = &input[1..];
             false
@@ -100,36 +106,36 @@ impl PxarExcludePattern {
         let pattern = CString::new(input)?;
         let split_pattern = split_at_slash(&pattern);
 
-        Ok(Some(PxarExcludePattern {
+        Ok(Some(MatchPattern {
             pattern,
-            match_exclude,
+            match_positive,
             match_dir_only,
             split_pattern,
         }))
     }
 
-    pub fn get_front_pattern(&self) -> PxarExcludePattern {
+    pub fn get_front_pattern(&self) -> MatchPattern {
         let pattern = split_at_slash(&self.split_pattern.0);
-        PxarExcludePattern {
+        MatchPattern {
             pattern: self.split_pattern.0.clone(),
-            match_exclude: self.match_exclude,
+            match_positive: self.match_positive,
             match_dir_only: self.match_dir_only,
             split_pattern: pattern,
         }
     }
 
-    pub fn get_rest_pattern(&self) -> PxarExcludePattern {
+    pub fn get_rest_pattern(&self) -> MatchPattern {
         let pattern = split_at_slash(&self.split_pattern.1);
-        PxarExcludePattern {
+        MatchPattern {
             pattern: self.split_pattern.1.clone(),
-            match_exclude: self.match_exclude,
+            match_positive: self.match_positive,
             match_dir_only: self.match_dir_only,
             split_pattern: pattern,
         }
     }
 
     pub fn dump(&self) {
-        match (self.match_exclude, self.match_dir_only) {
+        match (self.match_positive, self.match_dir_only) {
             (true, true) => println!("{:#?}/", self.pattern),
             (true, false) => println!("{:#?}", self.pattern),
             (false, true) => println!("!{:#?}/", self.pattern),
@@ -142,14 +148,16 @@ impl PxarExcludePattern {
         let (front, _) = &self.split_pattern;
 
         let fnmatch_res = unsafe {
-            fnmatch(front.as_ptr() as *const libc::c_char, filename.as_ptr() as *const libc::c_char, 0)
+            let front_ptr = front.as_ptr() as *const libc::c_char;
+            let filename_ptr = filename.as_ptr() as *const libc::c_char;
+            fnmatch(front_ptr, filename_ptr , 0)
         };
         // TODO error cases
         if fnmatch_res == 0 {
-            res = if self.match_exclude {
+            res = if self.match_positive {
-                MatchType::PartialExclude
+                MatchType::PartialPositive
             } else {
-                MatchType::PartialInclude
+                MatchType::PartialNegative
             };
         }
 
@@ -159,14 +167,16 @@ impl PxarExcludePattern {
             CString::new(&self.pattern.to_bytes()[..]).unwrap()
         };
         let fnmatch_res = unsafe {
-            fnmatch(full.as_ptr() as *const libc::c_char, filename.as_ptr() as *const libc::c_char, 0)
+            let full_ptr = full.as_ptr() as *const libc::c_char;
+            let filename_ptr = filename.as_ptr() as *const libc::c_char;
+            fnmatch(full_ptr, filename_ptr, 0)
         };
         // TODO error cases
         if fnmatch_res == 0 {
-            res = if self.match_exclude {
+            res = if self.match_positive {
-                MatchType::Exclude
+                MatchType::Positive
             } else {
-                MatchType::Include
+                MatchType::Negative
             };
         }
 
@@ -174,7 +184,7 @@ impl PxarExcludePattern {
             res = MatchType::None;
         }
 
-        if !is_dir && (res == MatchType::PartialInclude || res == MatchType::PartialExclude) {
+        if !is_dir && (res == MatchType::PartialPositive || res == MatchType::PartialNegative) {
             res = MatchType::None;
         }
 

src/pxar/sequential_decoder.rs

@@ -7,7 +7,7 @@ use endian_trait::Endian;
 
 use super::flags;
 use super::format_definition::*;
-use super::exclude_pattern::*;
+use super::match_pattern::*;
 use super::dir_stack::*;
 
 use std::io::{Read, Write};
@@ -630,14 +630,14 @@ impl <'a, R: Read, F: Fn(&Path) -> Result<(), Error>> SequentialDecoder<'a, R, F
         entry: PxarEntry,
         filename: &OsStr,
         matched: MatchType,
-        match_pattern: &Vec<PxarExcludePattern>,
+        match_pattern: &Vec<MatchPattern>,
     ) -> Result<(), Error> {
         let (mut head, attr) = self.read_attributes()
             .map_err(|err| format_err!("Reading of directory attributes failed - {}", err))?;
 
         let dir = PxarDir::new(filename, entry, attr);
         dirs.push(dir);
-        if matched == MatchType::Include {
+        if matched == MatchType::Positive {
             dirs.create_all_dirs(!self.allow_existing_dirs)?;
         }
 
@@ -674,7 +674,7 @@ impl <'a, R: Read, F: Fn(&Path) -> Result<(), Error>> SequentialDecoder<'a, R, F
     pub fn restore(
         &mut self,
         path: &Path,
-        match_pattern: &Vec<PxarExcludePattern>
+        match_pattern: &Vec<MatchPattern>
     ) -> Result<(), Error> {
 
         let _ = std::fs::create_dir(path);
@@ -685,7 +685,7 @@ impl <'a, R: Read, F: Fn(&Path) -> Result<(), Error>> SequentialDecoder<'a, R, F
         let mut dirs = PxarDirStack::new(fd);
         // An empty match pattern list indicates to restore the full archive.
         let matched = if match_pattern.len() == 0 {
-            MatchType::Include
+            MatchType::Positive
         } else {
             MatchType::None
         };
@@ -725,7 +725,7 @@ impl <'a, R: Read, F: Fn(&Path) -> Result<(), Error>> SequentialDecoder<'a, R, F
         dirs: &mut PxarDirStack,
         filename: &OsStr,
         parent_matched: MatchType,
-        match_pattern: &Vec<PxarExcludePattern>,
+        match_pattern: &Vec<MatchPattern>,
     ) -> Result<(), Error> {
         let relative_path = dirs.as_path_buf();
         let full_path = base_path.join(&relative_path).join(filename);
@@ -751,24 +751,16 @@ impl <'a, R: Read, F: Fn(&Path) -> Result<(), Error>> SequentialDecoder<'a, R, F
         let mut matched = parent_matched;
         if match_pattern.len() > 0 {
             match match_filename(filename, entry.mode as u32 & libc::S_IFMT == libc::S_IFDIR, match_pattern) {
-                (MatchType::Include, pattern) => {
-                    matched = MatchType::Include;
-                    child_pattern = pattern;
-                },
                 (MatchType::None, _) => matched = MatchType::None,
-                (MatchType::Exclude, _) => matched = MatchType::Exclude,
+                (MatchType::Negative, _) => matched = MatchType::Negative,
-                (MatchType::PartialExclude, pattern) => {
+                (match_type, pattern) => {
-                    matched = MatchType::PartialExclude;
+                    matched = match_type;
-                    child_pattern = pattern;
-                },
-                (MatchType::PartialInclude, pattern) => {
-                    matched = MatchType::PartialInclude;
                     child_pattern = pattern;
                 },
             }
         }
 
-        let fd = if matched == MatchType::Include {
+        let fd = if matched == MatchType::Positive {
             Some(dirs.create_all_dirs(!self.allow_existing_dirs)?)
         } else {
             None
@@ -1037,8 +1029,8 @@ impl <'a, R: Read, F: Fn(&Path) -> Result<(), Error>> SequentialDecoder<'a, R, F
 fn match_filename(
     filename: &OsStr,
     is_dir: bool,
-    match_pattern: &Vec<PxarExcludePattern>
+    match_pattern: &Vec<MatchPattern>
-) ->  (MatchType, Vec<PxarExcludePattern>) {
+) ->  (MatchType, Vec<MatchPattern>) {
     let mut child_pattern = Vec::new();
     let mut match_state = MatchType::None;
     // read_filename() checks for nul bytes, so it is save to unwrap here
@@ -1047,22 +1039,21 @@ fn match_filename(
     for pattern in match_pattern {
         match pattern.matches_filename(&name, is_dir) {
             MatchType::None =>  {},
-            // The logic is inverted here, since PxarExcludePattern assumes excludes not includes
+            MatchType::Positive => {
-            MatchType::Exclude => {
+                match_state = MatchType::Positive;
-                match_state = MatchType::Include;
+                let incl_pattern = MatchPattern::from_line(b"**/*").unwrap().unwrap();
-                let incl_pattern = PxarExcludePattern::from_line(b"**/*").unwrap().unwrap();
                 child_pattern.push(incl_pattern.get_rest_pattern());
             },
-            MatchType::Include =>  match_state = MatchType::Exclude,
+            MatchType::Negative =>  match_state = MatchType::Negative,
-            MatchType::PartialExclude =>  {
+            MatchType::PartialPositive =>  {
-                if match_state != MatchType::Include && match_state != MatchType::Exclude {
+                if match_state != MatchType::Negative && match_state != MatchType::Positive {
-                    match_state = MatchType::PartialInclude;
+                    match_state = MatchType::PartialPositive;
                 }
                 child_pattern.push(pattern.get_rest_pattern());
             },
-            MatchType::PartialInclude =>  {
+            MatchType::PartialNegative =>  {
-                if match_state == MatchType::PartialInclude {
+                if match_state == MatchType::PartialPositive {
-                    match_state = MatchType::PartialExclude;
+                    match_state = MatchType::PartialNegative;
                 }
                 child_pattern.push(pattern.get_rest_pattern());
             },