Re: [PATCH v1 proxmox-backup 1/2] datastore: prefer standard library over custom unsafe code

[Christian Ebner <c.ebner@proxmox.com>]

On 8/7/26 11:58 AM, Robert Obkircher wrote:
> The allocation is unnecessary overhead, and the unsafe code in
> read_exact_allocated looks a bit dangerous.
> 
> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
> ---
>   pbs-datastore/src/chunk_store.rs | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
> index 6d2ffdbde..3f637730c 100644
> --- a/pbs-datastore/src/chunk_store.rs
> +++ b/pbs-datastore/src/chunk_store.rs
> @@ -1,3 +1,4 @@
> +use std::io::Read;
>   use std::os::unix::fs::MetadataExt;
>   use std::os::unix::io::AsRawFd;
>   use std::path::{Path, PathBuf};
> @@ -10,7 +11,6 @@ use tracing::{info, warn};
>   
>   use pbs_api_types::{DatastoreFSyncLevel, GarbageCollectionStatus};
>   use pbs_config::BackupLockGuard;
> -use proxmox_io::ReadExt;
>   use proxmox_s3_client::S3Client;
>   use proxmox_sys::fs::{CreateOptions, create_dir, create_path, file_type_from_file_stat};
>   use proxmox_sys::process_locker::{
> @@ -708,8 +708,11 @@ impl ChunkStore {
>                   }
>               } else if chunk.is_encrypted() {
>                   // incoming chunk is encrypted, possible attack or hash collision!
> -                let mut existing_file = std::fs::File::open(&chunk_path)?;
> -                let magic = existing_file.read_exact_allocated(8)?;
> +
> +                let mut magic = [0u8; 8];
> +                std::fs::File::open(&chunk_path)
> +                    .and_then(|mut f| f.read_exact(&mut magic))
> +                    .map_err(|e| format_err!("Failed to read header of existing chunk '{digest_str}' on store '{name}: {e}"))?;

nit: this does not read the chunk header 
(DataBlobHeader/EncryptedDataBlobHeader), just the magic number. So 
maybe better to also refer to it in the error message as such.

>   
>                   // going from unencrypted to encrypted can never be right, since the digest
>                   // includes data derived from the encryption key