public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Max R. Carrara" <m.carrara@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [PATCH proxmox-backup v2 04/10] tape: rename `BlockHeader` and `BlockHeaderFlags`
Date: Fri, 21 Aug 2026 16:02:28 +0200	[thread overview]
Message-ID: <20260821140238.615302-5-m.carrara@proxmox.com> (raw)
In-Reply-To: <20260821140238.615302-1-m.carrara@proxmox.com>

... to `TapeBlock` and `TapeBlockFlags`, since `BlockHeader` is a bit
of a misnomer -- each tape block has a header followed by a data
payload, so it makes sense to just name it after what it is.

Also adapt the docstring for `BlockHeader`.

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
 pbs-tape/src/blocked_reader.rs | 23 ++++++++++-------------
 pbs-tape/src/blocked_writer.rs | 23 ++++++++++-------------
 pbs-tape/src/lib.rs            |  2 +-
 pbs-tape/src/tape_block.rs     | 23 +++++++++++------------
 4 files changed, 32 insertions(+), 39 deletions(-)

diff --git a/pbs-tape/src/blocked_reader.rs b/pbs-tape/src/blocked_reader.rs
index 22803371c..3e3f7095c 100644
--- a/pbs-tape/src/blocked_reader.rs
+++ b/pbs-tape/src/blocked_reader.rs
@@ -1,7 +1,7 @@
 use std::io::Read;
 
 use crate::{
-    BlockHeader, BlockHeaderFlags, BlockRead, BlockReadError, PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0,
+    BlockRead, BlockReadError, PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0, TapeBlock, TapeBlockFlags,
     TapeRead,
 };
 
@@ -18,7 +18,7 @@ use crate::{
 /// the end of the stream).
 pub struct BlockedReader<R> {
     reader: R,
-    buffer: Box<BlockHeader>,
+    buffer: Box<TapeBlock>,
     seq_nr: u32,
     found_end_marker: bool,
     incomplete: bool,
@@ -33,7 +33,7 @@ impl<R: BlockRead> BlockedReader<R> {
     /// This tries to read the first block. Please inspect the error
     /// to detect EOF and EOT.
     pub fn open(mut reader: R) -> Result<Self, BlockReadError> {
-        let mut buffer = BlockHeader::new();
+        let mut buffer = TapeBlock::new();
 
         Self::read_block_frame(&mut buffer, &mut reader)?;
 
@@ -43,7 +43,7 @@ impl<R: BlockRead> BlockedReader<R> {
         let mut got_eod = false;
 
         if found_end_marker {
-            incomplete = buffer.flags.contains(BlockHeaderFlags::INCOMPLETE);
+            incomplete = buffer.flags.contains(TapeBlockFlags::INCOMPLETE);
             Self::consume_eof_marker(&mut reader)?;
             got_eod = true;
         }
@@ -60,7 +60,7 @@ impl<R: BlockRead> BlockedReader<R> {
         })
     }
 
-    fn check_buffer(buffer: &BlockHeader, seq_nr: u32) -> Result<(usize, bool), std::io::Error> {
+    fn check_buffer(buffer: &TapeBlock, seq_nr: u32) -> Result<(usize, bool), std::io::Error> {
         if buffer.magic != PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0 {
             proxmox_lang::io_bail!(
                 "got tape block with unknown magic number - not written by PBS or incompatible LTO version"
@@ -76,7 +76,7 @@ impl<R: BlockRead> BlockedReader<R> {
         }
 
         let size = buffer.size();
-        let found_end_marker = buffer.flags.contains(BlockHeaderFlags::END_OF_STREAM);
+        let found_end_marker = buffer.flags.contains(TapeBlockFlags::END_OF_STREAM);
 
         if size > buffer.payload.len() {
             proxmox_lang::io_bail!(
@@ -91,17 +91,14 @@ impl<R: BlockRead> BlockedReader<R> {
         Ok((size, found_end_marker))
     }
 
-    fn read_block_frame(buffer: &mut BlockHeader, reader: &mut R) -> Result<(), BlockReadError> {
+    fn read_block_frame(buffer: &mut TapeBlock, reader: &mut R) -> Result<(), BlockReadError> {
         let data = unsafe {
-            std::slice::from_raw_parts_mut(
-                (buffer as *mut BlockHeader) as *mut u8,
-                BlockHeader::SIZE,
-            )
+            std::slice::from_raw_parts_mut((buffer as *mut TapeBlock) as *mut u8, TapeBlock::SIZE)
         };
 
         let bytes = reader.read_block(data)?;
 
-        if bytes != BlockHeader::SIZE {
+        if bytes != TapeBlock::SIZE {
             return Err(proxmox_lang::io_format_err!("got wrong block size").into());
         }
 
@@ -147,7 +144,7 @@ impl<R: BlockRead> BlockedReader<R> {
         if found_end_marker {
             // consume EOF mark
             self.found_end_marker = true;
-            self.incomplete = self.buffer.flags.contains(BlockHeaderFlags::INCOMPLETE);
+            self.incomplete = self.buffer.flags.contains(TapeBlockFlags::INCOMPLETE);
             Self::consume_eof_marker(&mut self.reader)?;
             self.got_eod = true;
         }
diff --git a/pbs-tape/src/blocked_writer.rs b/pbs-tape/src/blocked_writer.rs
index 7380af243..9aba7b832 100644
--- a/pbs-tape/src/blocked_writer.rs
+++ b/pbs-tape/src/blocked_writer.rs
@@ -1,6 +1,6 @@
 use proxmox_io::vec;
 
-use crate::{BlockHeader, BlockHeaderFlags, BlockWrite, TapeWrite};
+use crate::{BlockWrite, TapeBlock, TapeBlockFlags, TapeWrite};
 
 /// Assemble and write blocks of data
 ///
@@ -9,7 +9,7 @@ use crate::{BlockHeader, BlockHeaderFlags, BlockWrite, TapeWrite};
 /// to the underlying writer.
 pub struct BlockedWriter<W: BlockWrite> {
     writer: W,
-    buffer: Box<BlockHeader>,
+    buffer: Box<TapeBlock>,
     buffer_pos: usize,
     seq_nr: u32,
     logical_end_of_media: bool,
@@ -36,7 +36,7 @@ impl<W: BlockWrite> BlockedWriter<W> {
     pub fn new(writer: W) -> Self {
         Self {
             writer,
-            buffer: BlockHeader::new(),
+            buffer: TapeBlock::new(),
             buffer_pos: 0,
             seq_nr: 0,
             logical_end_of_media: false,
@@ -45,12 +45,9 @@ impl<W: BlockWrite> BlockedWriter<W> {
         }
     }
 
-    fn write_block(buffer: &BlockHeader, writer: &mut W) -> Result<bool, std::io::Error> {
+    fn write_block(buffer: &TapeBlock, writer: &mut W) -> Result<bool, std::io::Error> {
         let data = unsafe {
-            std::slice::from_raw_parts(
-                (buffer as *const BlockHeader) as *const u8,
-                BlockHeader::SIZE,
-            )
+            std::slice::from_raw_parts((buffer as *const TapeBlock) as *const u8, TapeBlock::SIZE)
         };
         writer.write_block(data)
     }
@@ -77,7 +74,7 @@ impl<W: BlockWrite> BlockedWriter<W> {
         let rest = rest - bytes;
 
         if rest == 0 {
-            self.buffer.flags = BlockHeaderFlags::empty();
+            self.buffer.flags = TapeBlockFlags::empty();
             self.buffer.set_size(self.buffer.payload.len());
             self.buffer.set_seq_nr(self.seq_nr);
             self.seq_nr += 1;
@@ -86,7 +83,7 @@ impl<W: BlockWrite> BlockedWriter<W> {
                 self.logical_end_of_media = true;
             }
             self.buffer_pos = 0;
-            self.bytes_written += BlockHeader::SIZE;
+            self.bytes_written += TapeBlock::SIZE;
         } else {
             self.buffer_pos += bytes;
         }
@@ -116,14 +113,14 @@ impl<W: BlockWrite> TapeWrite for BlockedWriter<W> {
     /// END_OF_STREAM flag.
     fn finish(&mut self, incomplete: bool) -> Result<bool, std::io::Error> {
         vec::clear(&mut self.buffer.payload[self.buffer_pos..]);
-        self.buffer.flags = BlockHeaderFlags::END_OF_STREAM;
+        self.buffer.flags = TapeBlockFlags::END_OF_STREAM;
         if incomplete {
-            self.buffer.flags |= BlockHeaderFlags::INCOMPLETE;
+            self.buffer.flags |= TapeBlockFlags::INCOMPLETE;
         }
         self.buffer.set_size(self.buffer_pos);
         self.buffer.set_seq_nr(self.seq_nr);
         self.seq_nr += 1;
-        self.bytes_written += BlockHeader::SIZE;
+        self.bytes_written += TapeBlock::SIZE;
         let leom = Self::write_block(&self.buffer, &mut self.writer)?;
         self.write_eof()?;
         Ok(leom)
diff --git a/pbs-tape/src/lib.rs b/pbs-tape/src/lib.rs
index 6cff175dd..1b6dc94cc 100644
--- a/pbs-tape/src/lib.rs
+++ b/pbs-tape/src/lib.rs
@@ -20,7 +20,7 @@ mod blocked_writer;
 pub use blocked_writer::BlockedWriter;
 
 mod tape_block;
-pub(crate) use tape_block::{BlockHeader, BlockHeaderFlags};
+pub(crate) use tape_block::{TapeBlock, TapeBlockFlags};
 
 mod tape_write;
 pub use tape_write::*;
diff --git a/pbs-tape/src/tape_block.rs b/pbs-tape/src/tape_block.rs
index 4f9c2192f..4ab1f5ce9 100644
--- a/pbs-tape/src/tape_block.rs
+++ b/pbs-tape/src/tape_block.rs
@@ -3,23 +3,22 @@ use bitflags::bitflags;
 use crate::PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0;
 use crate::PROXMOX_TAPE_BLOCK_SIZE;
 
-/// Tape Block Header with data payload
+/// A [`TapeBlock`] consists of a tape header followed by a data payload.
 ///
 /// All tape files are written as sequence of blocks.
 ///
-/// Note: this struct is large, never put this on the stack!
-/// so we use an unsized type to avoid that.
+/// Note: This struct is a dynamically sized type and can therefore only ever
+/// exist as a heap-allocated value.
 ///
-/// Tape data block are always read/written with a fixed size
-/// (`PROXMOX_TAPE_BLOCK_SIZE`). But they may contain less data, so the
-/// header has an additional size field. For streams of blocks, there
-/// is a sequence number (`seq_nr`) which may be use for additional
-/// error checking.
+/// Tape blocks are always read/written with a fixed size
+/// (`PROXMOX_TAPE_BLOCK_SIZE`). However, since they may contain less data, the
+/// header has an additional size field. For streams of blocks, there is a
+/// sequence number (`seq_nr`) which may be used for additional error checking.
 #[repr(C, packed)]
-pub struct BlockHeader {
+pub struct TapeBlock {
     /// fixed value `PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0`
     pub magic: [u8; 8],
-    pub flags: BlockHeaderFlags,
+    pub flags: TapeBlockFlags,
     /// size as 3 bytes unsigned, little endian
     pub size: [u8; 3],
     /// block sequence number
@@ -30,7 +29,7 @@ pub struct BlockHeader {
 bitflags! {
     /// Header flags (e.g. `END_OF_STREAM` or `INCOMPLETE`)
     #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
-    pub struct BlockHeaderFlags: u8 {
+    pub struct TapeBlockFlags: u8 {
         /// Marks the last block in a stream.
         const END_OF_STREAM = 0b00000001;
         /// Mark multivolume streams (when set in the last block)
@@ -38,7 +37,7 @@ bitflags! {
     }
 }
 
-impl BlockHeader {
+impl TapeBlock {
     pub const SIZE: usize = PROXMOX_TAPE_BLOCK_SIZE;
 
     /// Allocates a new instance on the heap
-- 
2.47.3





  parent reply	other threads:[~2026-08-21 14:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21 14:02 [PATCH proxmox{,-backup} v2 00/10] Fix Undefined Behavior in Tape Block Header Deallocation Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox v2 01/10] proxmox-alloc: introduce proxmox-alloc with `LayoutAwareBox<T>` type Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox v2 02/10] proxmox-alloc: document undefined behavior regarding custom allocs Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 03/10] tape: move tape block structs into separate file module Max R. Carrara
2026-08-21 14:02 ` Max R. Carrara [this message]
2026-08-21 14:02 ` [PATCH proxmox-backup v2 05/10] tape: blocked_{reader,writer}: rename `buffer` to `tape_block` Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 06/10] tape: tape block: represent tape block header with its own struct Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 07/10] tape: tape block: make `payload` field private Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 08/10] tape: blocked_{reader,writer}: remove haphazard `unsafe` blocks Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 09/10] tape: tape block: fix undefined behavior on tape block deallocation Max R. Carrara
2026-08-21 14:02 ` [PATCH proxmox-backup v2 10/10] tape: sgutils2: fix undefined behavior in dealloc of buffer Max R. Carrara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260821140238.615302-5-m.carrara@proxmox.com \
    --to=m.carrara@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal