all lists on 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 05/10] tape: blocked_{reader,writer}: rename `buffer` to `tape_block`
Date: Fri, 21 Aug 2026 16:02:29 +0200	[thread overview]
Message-ID: <20260821140238.615302-6-m.carrara@proxmox.com> (raw)
In-Reply-To: <20260821140238.615302-1-m.carrara@proxmox.com>

... since a tape block does not really represent a plain buffer.

Also rename any private functions mentioning `buffer` along the way.

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
 pbs-tape/src/blocked_reader.rs | 63 +++++++++++++++++++---------------
 pbs-tape/src/blocked_writer.rs | 35 ++++++++++---------
 2 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/pbs-tape/src/blocked_reader.rs b/pbs-tape/src/blocked_reader.rs
index 3e3f7095c..ef3f8d217 100644
--- a/pbs-tape/src/blocked_reader.rs
+++ b/pbs-tape/src/blocked_reader.rs
@@ -18,7 +18,7 @@ use crate::{
 /// the end of the stream).
 pub struct BlockedReader<R> {
     reader: R,
-    buffer: Box<TapeBlock>,
+    tape_block: Box<TapeBlock>,
     seq_nr: u32,
     found_end_marker: bool,
     incomplete: bool,
@@ -33,24 +33,24 @@ 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 = TapeBlock::new();
+        let mut tape_block = TapeBlock::new();
 
-        Self::read_block_frame(&mut buffer, &mut reader)?;
+        Self::read_block_frame(&mut tape_block, &mut reader)?;
 
-        let (_size, found_end_marker) = Self::check_buffer(&buffer, 0)?;
+        let (_size, found_end_marker) = Self::check_tape_block(&tape_block, 0)?;
 
         let mut incomplete = false;
         let mut got_eod = false;
 
         if found_end_marker {
-            incomplete = buffer.flags.contains(TapeBlockFlags::INCOMPLETE);
+            incomplete = tape_block.flags.contains(TapeBlockFlags::INCOMPLETE);
             Self::consume_eof_marker(&mut reader)?;
             got_eod = true;
         }
 
         Ok(Self {
             reader,
-            buffer,
+            tape_block,
             found_end_marker,
             incomplete,
             got_eod,
@@ -60,29 +60,32 @@ impl<R: BlockRead> BlockedReader<R> {
         })
     }
 
-    fn check_buffer(buffer: &TapeBlock, seq_nr: u32) -> Result<(usize, bool), std::io::Error> {
-        if buffer.magic != PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0 {
+    fn check_tape_block(
+        tape_block: &TapeBlock,
+        seq_nr: u32,
+    ) -> Result<(usize, bool), std::io::Error> {
+        if tape_block.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"
             );
         }
 
-        if seq_nr != buffer.seq_nr() {
+        if seq_nr != tape_block.seq_nr() {
             proxmox_lang::io_bail!(
                 "detected tape block with wrong sequence number ({} != {})",
                 seq_nr,
-                buffer.seq_nr()
+                tape_block.seq_nr()
             )
         }
 
-        let size = buffer.size();
-        let found_end_marker = buffer.flags.contains(TapeBlockFlags::END_OF_STREAM);
+        let size = tape_block.size();
+        let found_end_marker = tape_block.flags.contains(TapeBlockFlags::END_OF_STREAM);
 
-        if size > buffer.payload.len() {
+        if size > tape_block.payload.len() {
             proxmox_lang::io_bail!(
                 "detected tape block with wrong payload size ({} > {}",
                 size,
-                buffer.payload.len()
+                tape_block.payload.len()
             );
         } else if size == 0 && !found_end_marker {
             proxmox_lang::io_bail!("detected tape block with zero payload size");
@@ -91,9 +94,12 @@ impl<R: BlockRead> BlockedReader<R> {
         Ok((size, found_end_marker))
     }
 
-    fn read_block_frame(buffer: &mut TapeBlock, reader: &mut R) -> Result<(), BlockReadError> {
+    fn read_block_frame(tape_block: &mut TapeBlock, reader: &mut R) -> Result<(), BlockReadError> {
         let data = unsafe {
-            std::slice::from_raw_parts_mut((buffer as *mut TapeBlock) as *mut u8, TapeBlock::SIZE)
+            std::slice::from_raw_parts_mut(
+                (tape_block as *mut TapeBlock) as *mut u8,
+                TapeBlock::SIZE,
+            )
         };
 
         let bytes = reader.read_block(data)?;
@@ -120,11 +126,11 @@ impl<R: BlockRead> BlockedReader<R> {
     }
 
     fn read_block(&mut self, check_end_marker: bool) -> Result<usize, std::io::Error> {
-        match Self::read_block_frame(&mut self.buffer, &mut self.reader) {
+        match Self::read_block_frame(&mut self.tape_block, &mut self.reader) {
             Ok(()) => { /* ok */ }
             Err(BlockReadError::EndOfFile) => {
                 self.got_eod = true;
-                self.read_pos = self.buffer.payload.len();
+                self.read_pos = self.tape_block.payload.len();
                 if !self.found_end_marker && check_end_marker {
                     proxmox_lang::io_bail!("detected tape stream without end marker");
                 }
@@ -138,13 +144,13 @@ impl<R: BlockRead> BlockedReader<R> {
             }
         }
 
-        let (size, found_end_marker) = Self::check_buffer(&self.buffer, self.seq_nr)?;
+        let (size, found_end_marker) = Self::check_tape_block(&self.tape_block, self.seq_nr)?;
         self.seq_nr += 1;
 
         if found_end_marker {
             // consume EOF mark
             self.found_end_marker = true;
-            self.incomplete = self.buffer.flags.contains(TapeBlockFlags::INCOMPLETE);
+            self.incomplete = self.tape_block.flags.contains(TapeBlockFlags::INCOMPLETE);
             Self::consume_eof_marker(&mut self.reader)?;
             self.got_eod = true;
         }
@@ -179,8 +185,8 @@ impl<R: BlockRead> TapeRead for BlockedReader<R> {
     // stream has no end marker.
     fn skip_data(&mut self) -> Result<usize, std::io::Error> {
         let mut bytes = 0;
-        let buffer_size = self.buffer.size();
-        let rest = (buffer_size as isize) - (self.read_pos as isize);
+        let tape_block_size = self.tape_block.size();
+        let rest = (tape_block_size as isize) - (self.read_pos as isize);
         if rest > 0 {
             bytes = rest as usize;
         }
@@ -199,19 +205,19 @@ impl<R: BlockRead> Read for BlockedReader<R> {
             proxmox_lang::io_bail!("detected read after error - internal error");
         }
 
-        let mut buffer_size = self.buffer.size();
-        let mut rest = (buffer_size as isize) - (self.read_pos as isize);
+        let mut tape_block_size = self.tape_block.size();
+        let mut rest = (tape_block_size as isize) - (self.read_pos as isize);
 
         if rest <= 0 && !self.got_eod {
             // try to refill buffer
-            buffer_size = match self.read_block(true) {
+            tape_block_size = match self.read_block(true) {
                 Ok(len) => len,
                 err => {
                     self.read_error = true;
                     return err;
                 }
             };
-            rest = buffer_size as isize;
+            rest = tape_block_size as isize;
         }
 
         if rest <= 0 {
@@ -222,8 +228,9 @@ impl<R: BlockRead> Read for BlockedReader<R> {
             } else {
                 rest as usize
             };
-            buffer[..copy_len]
-                .copy_from_slice(&self.buffer.payload[self.read_pos..(self.read_pos + copy_len)]);
+            buffer[..copy_len].copy_from_slice(
+                &self.tape_block.payload[self.read_pos..(self.read_pos + copy_len)],
+            );
             self.read_pos += copy_len;
             Ok(copy_len)
         }
diff --git a/pbs-tape/src/blocked_writer.rs b/pbs-tape/src/blocked_writer.rs
index 9aba7b832..9aab0cbb0 100644
--- a/pbs-tape/src/blocked_writer.rs
+++ b/pbs-tape/src/blocked_writer.rs
@@ -9,7 +9,7 @@ use crate::{BlockWrite, TapeBlock, TapeBlockFlags, TapeWrite};
 /// to the underlying writer.
 pub struct BlockedWriter<W: BlockWrite> {
     writer: W,
-    buffer: Box<TapeBlock>,
+    tape_block: 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: TapeBlock::new(),
+            tape_block: TapeBlock::new(),
             buffer_pos: 0,
             seq_nr: 0,
             logical_end_of_media: false,
@@ -45,9 +45,12 @@ impl<W: BlockWrite> BlockedWriter<W> {
         }
     }
 
-    fn write_block(buffer: &TapeBlock, writer: &mut W) -> Result<bool, std::io::Error> {
+    fn write_block(tape_block: &TapeBlock, writer: &mut W) -> Result<bool, std::io::Error> {
         let data = unsafe {
-            std::slice::from_raw_parts((buffer as *const TapeBlock) as *const u8, TapeBlock::SIZE)
+            std::slice::from_raw_parts(
+                (tape_block as *const TapeBlock) as *const u8,
+                TapeBlock::SIZE,
+            )
         };
         writer.write_block(data)
     }
@@ -66,19 +69,19 @@ impl<W: BlockWrite> BlockedWriter<W> {
             return Ok(0);
         }
 
-        let rest = self.buffer.payload.len() - self.buffer_pos;
+        let rest = self.tape_block.payload.len() - self.buffer_pos;
         let bytes = if data.len() < rest { data.len() } else { rest };
-        self.buffer.payload[self.buffer_pos..(self.buffer_pos + bytes)]
+        self.tape_block.payload[self.buffer_pos..(self.buffer_pos + bytes)]
             .copy_from_slice(&data[..bytes]);
 
         let rest = rest - bytes;
 
         if rest == 0 {
-            self.buffer.flags = TapeBlockFlags::empty();
-            self.buffer.set_size(self.buffer.payload.len());
-            self.buffer.set_seq_nr(self.seq_nr);
+            self.tape_block.flags = TapeBlockFlags::empty();
+            self.tape_block.set_size(self.tape_block.payload.len());
+            self.tape_block.set_seq_nr(self.seq_nr);
             self.seq_nr += 1;
-            let leom = Self::write_block(&self.buffer, &mut self.writer)?;
+            let leom = Self::write_block(&self.tape_block, &mut self.writer)?;
             if leom {
                 self.logical_end_of_media = true;
             }
@@ -112,16 +115,16 @@ impl<W: BlockWrite> TapeWrite for BlockedWriter<W> {
     /// Note: This may write an empty block just including the
     /// 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 = TapeBlockFlags::END_OF_STREAM;
+        vec::clear(&mut self.tape_block.payload[self.buffer_pos..]);
+        self.tape_block.flags = TapeBlockFlags::END_OF_STREAM;
         if incomplete {
-            self.buffer.flags |= TapeBlockFlags::INCOMPLETE;
+            self.tape_block.flags |= TapeBlockFlags::INCOMPLETE;
         }
-        self.buffer.set_size(self.buffer_pos);
-        self.buffer.set_seq_nr(self.seq_nr);
+        self.tape_block.set_size(self.buffer_pos);
+        self.tape_block.set_seq_nr(self.seq_nr);
         self.seq_nr += 1;
         self.bytes_written += TapeBlock::SIZE;
-        let leom = Self::write_block(&self.buffer, &mut self.writer)?;
+        let leom = Self::write_block(&self.tape_block, &mut self.writer)?;
         self.write_eof()?;
         Ok(leom)
     }
-- 
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 ` [PATCH proxmox-backup v2 04/10] tape: rename `BlockHeader` and `BlockHeaderFlags` Max R. Carrara
2026-08-21 14:02 ` Max R. Carrara [this message]
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-6-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal