From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id AF5048AA9 for ; Tue, 22 Aug 2023 15:07:36 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8ED69D38F for ; Tue, 22 Aug 2023 15:07:06 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 22 Aug 2023 15:07:05 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id D8FEB41DA1 for ; Tue, 22 Aug 2023 15:07:04 +0200 (CEST) Date: Tue, 22 Aug 2023 15:07:03 +0200 From: Wolfgang Bumiller To: Gabriel Goller Cc: pbs-devel@lists.proxmox.com Message-ID: References: <20230821130826.147473-1-g.goller@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230821130826.147473-1-g.goller@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.103 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH pathpatterns v4] match_list: updated `matches()`, to only retrieve file mode if necessary X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Aug 2023 13:07:36 -0000 LGTM now, 1 minor nit below since the other patch needs another version anyway... On Mon, Aug 21, 2023 at 03:08:25PM +0200, Gabriel Goller wrote: > 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` (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 > --- > > 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/match_list.rs b/src/match_list.rs > index c5b14e0..69b1fdb 100644 > --- a/src/match_list.rs > +++ b/src/match_list.rs (...) > @@ -383,30 +438,48 @@ where > <&'a T as IntoIterator>::IntoIter: DoubleEndedIterator, > <&'a T as IntoIterator>::Item: MatchListEntry, > { Since we have a T here already... > - fn matches_do(&self, path: &[u8], file_mode: Option) -> Option { > + fn matches(&self, path: U, get_file_mode: G) -> Result, G::Error> maybe use `P` for the `path` ;-) > + 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) -> Option { > + fn matches_exact(&self, path: U, get_file_mode: G) -> Result, G::Error> ^ here as well > + 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) > } > } >