From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 9E25F1FF13A for ; Wed, 22 Jul 2026 13:31:47 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 6CA7B214CF; Wed, 22 Jul 2026 13:31:47 +0200 (CEST) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [PATCH proxmox-backup v2 2/5] datastore: data blob: refactor decoding method From: Robert Obkircher To: Christian Ebner In-Reply-To: <20260507130135.589100-3-c.ebner@proxmox.com> References: <20260507130135.589100-1-c.ebner@proxmox.com> <20260507130135.589100-3-c.ebner@proxmox.com> Date: Wed, 22 Jul 2026 13:31:06 +0200 Message-Id: <178471986678.177215.16619713207450916356.b4-review@b4> X-Mailer: b4 0.16-dev X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784719847021 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.237 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: AV4BKCOOJ76RP6FSDYXARCULNWROHTAD X-Message-ID-Hash: AV4BKCOOJ76RP6FSDYXARCULNWROHTAD X-MailFrom: r.obkircher@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pbs-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: > Improve code style and readability by using a single match statement > instead of chained if statements and deduplicate the common digest > verificaton, performed on the decoded blob data. > > Signed-off-by: Christian Ebner > > diff --git a/pbs-datastore/src/data_blob.rs b/pbs-datastore/src/data_blob.rs > index 465bcb280..9fd791af5 100644 > --- a/pbs-datastore/src/data_blob.rs > +++ b/pbs-datastore/src/data_blob.rs > @@ -180,57 +180,57 @@ impl DataBlob { > config: Option<&CryptConfig>, > digest: Option<&[u8; 32]>, > ) -> Result, Error> { > - let magic = self.magic(); > + let magic = *self.magic(); > > - if magic == &UNCOMPRESSED_BLOB_MAGIC_1_0 { > - let data_start = std::mem::size_of::(); > - let data = self.raw_data[data_start..].to_vec(); > - if let Some(digest) = digest { > - Self::verify_digest(&data, None, digest)?; > + let (data, crypt_config) = match magic { > + UNCOMPRESSED_BLOB_MAGIC_1_0 => { > + let data_start = std::mem::size_of::(); > + let data = self.raw_data[data_start..].to_vec(); > + (data, None) > } > - Ok(data) > - } else if magic == &COMPRESSED_BLOB_MAGIC_1_0 { > - let data_start = std::mem::size_of::(); > - let mut reader = &self.raw_data[data_start..]; > - let data = zstd::stream::decode_all(&mut reader)?; > - // zstd::block::decompress is about 10% slower > - // let data = zstd::block::decompress(&self.raw_data[data_start..], MAX_BLOB_SIZE)?; > - if let Some(digest) = digest { > - Self::verify_digest(&data, None, digest)?; > + COMPRESSED_BLOB_MAGIC_1_0 => { > + let data_start = std::mem::size_of::(); > + let mut reader = &self.raw_data[data_start..]; > + let data = zstd::stream::decode_all(&mut reader)?; > + // zstd::block::decompress is about 10% slower > + // let data = zstd::block::decompress(&self.raw_data[data_start..], MAX_BLOB_SIZE)?; > + (data, None) > } > - Ok(data) > - } else if magic == &ENCR_COMPR_BLOB_MAGIC_1_0 || magic == &ENCRYPTED_BLOB_MAGIC_1_0 { > - let header_len = std::mem::size_of::(); > - let head = unsafe { > - (&self.raw_data[..header_len]).read_le_value::()? > - }; > + ENCR_COMPR_BLOB_MAGIC_1_0 | ENCRYPTED_BLOB_MAGIC_1_0 => { > + let header_len = std::mem::size_of::(); > + let head = unsafe { > + (&self.raw_data[..header_len]).read_le_value::()? > + }; > > - if let Some(config) = config { > - let data = if magic == &ENCR_COMPR_BLOB_MAGIC_1_0 { > - Self::decode_compressed_chunk( > - config, > - &self.raw_data[header_len..], > - &head.iv, > - &head.tag, > - )? > + if let Some(config) = config { nit: this could be `let Some(config) = config else { bail!(..); };` to reduce indentation. -- Robert Obkircher