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 B07961FF0AA for ; Mon, 07 Sep 2026 13:44:43 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id C1EB22156E; Mon, 07 Sep 2026 13:44:41 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 07 Sep 2026 13:44:33 +0200 Message-Id: Subject: Re: [PATCH proxmox v2 02/10] proxmox-alloc: document undefined behavior regarding custom allocs To: "Robert Obkircher" From: "Max R. Carrara" X-Mailer: aerc 0.18.2-0-ge037c095a049 References: <20260821140238.615302-1-m.carrara@proxmox.com> <20260821140238.615302-3-m.carrara@proxmox.com> <178836583424.294173.6954096291117410170.b4-review@b4> In-Reply-To: <178836583424.294173.6954096291117410170.b4-review@b4> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788781467178 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.620 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_MED -2.3 Sender listed at https://www.dnswl.org/, medium 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: OEJUQSTYSSW5NK364EQRSFYQXUMN2I6R X-Message-ID-Hash: OEJUQSTYSSW5NK364EQRSFYQXUMN2I6R X-MailFrom: m.carrara@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: On Wed Sep 2, 2026 at 6:17 PM CEST, Robert Obkircher wrote: > > ... 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 > > > > diff --git a/pbs-tape/src/blocked_reader.rs b/pbs-tape/src/blocked_read= er.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, Ta= peBlock, TapeBlockFlags, > > TapeRead, > > }; > > > > @@ -18,7 +18,7 @@ use crate::{ > > /// the end of the stream). > > pub struct BlockedReader { > > reader: R, > > - buffer: Box, > > + buffer: Box, > > seq_nr: u32, > > found_end_marker: bool, > > incomplete: bool, > > @@ -33,7 +33,7 @@ impl BlockedReader { > > /// This tries to read the first block. Please inspect the error > > /// to detect EOF and EOT. > > pub fn open(mut reader: R) -> Result { > > - let mut buffer =3D BlockHeader::new(); > > + let mut buffer =3D TapeBlock::new(); > > > > Self::read_block_frame(&mut buffer, &mut reader)?; > > > > @@ -43,7 +43,7 @@ impl BlockedReader { > > let mut got_eod =3D false; > > > > if found_end_marker { > > - incomplete =3D buffer.flags.contains(BlockHeaderFlags::INC= OMPLETE); > > + incomplete =3D buffer.flags.contains(TapeBlockFlags::INCOM= PLETE); > > Self::consume_eof_marker(&mut reader)?; > > got_eod =3D true; > > } > > @@ -60,7 +60,7 @@ impl BlockedReader { > > }) > > } > > > > - fn check_buffer(buffer: &BlockHeader, seq_nr: u32) -> Result<(usiz= e, bool), std::io::Error> { > > + fn check_buffer(buffer: &TapeBlock, seq_nr: u32) -> Result<(usize,= bool), std::io::Error> { > > if buffer.magic !=3D PROXMOX_TAPE_BLOCK_HEADER_MAGIC_1_0 { > > proxmox_lang::io_bail!( > > "got tape block with unknown magic number - not writte= n by PBS or incompatible LTO version" > > @@ -76,7 +76,7 @@ impl BlockedReader { > > } > > > > let size =3D buffer.size(); > > - let found_end_marker =3D buffer.flags.contains(BlockHeaderFlag= s::END_OF_STREAM); > > + let found_end_marker =3D buffer.flags.contains(TapeBlockFlags:= :END_OF_STREAM); > > > > if size > buffer.payload.len() { > > proxmox_lang::io_bail!( > > @@ -91,17 +91,14 @@ impl BlockedReader { > > Ok((size, found_end_marker)) > > } > > > > - fn read_block_frame(buffer: &mut BlockHeader, reader: &mut R) -> R= esult<(), BlockReadError> { > > + fn read_block_frame(buffer: &mut TapeBlock, reader: &mut R) -> Res= ult<(), BlockReadError> { > > let data =3D 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 =3D reader.read_block(data)?; > > > > - if bytes !=3D BlockHeader::SIZE { > > + if bytes !=3D TapeBlock::SIZE { > > return Err(proxmox_lang::io_format_err!("got wrong block s= ize").into()); > > } > > > > @@ -147,7 +144,7 @@ impl BlockedReader { > > if found_end_marker { > > // consume EOF mark > > self.found_end_marker =3D true; > > - self.incomplete =3D self.buffer.flags.contains(BlockHeader= Flags::INCOMPLETE); > > + self.incomplete =3D self.buffer.flags.contains(TapeBlockFl= ags::INCOMPLETE); > > Self::consume_eof_marker(&mut self.reader)?; > > self.got_eod =3D true; > > } > > diff --git a/pbs-tape/src/blocked_writer.rs b/pbs-tape/src/blocked_writ= er.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 { > > writer: W, > > - buffer: Box, > > + buffer: Box, > > buffer_pos: usize, > > seq_nr: u32, > > logical_end_of_media: bool, > > @@ -36,7 +36,7 @@ impl BlockedWriter { > > 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 BlockedWriter { > > } > > } > > > > - fn write_block(buffer: &BlockHeader, writer: &mut W) -> Result { > > + fn write_block(buffer: &TapeBlock, writer: &mut W) -> Result { > > let data =3D 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 BlockedWriter { > > let rest =3D rest - bytes; > > > > if rest =3D=3D 0 { > > - self.buffer.flags =3D BlockHeaderFlags::empty(); > > + self.buffer.flags =3D TapeBlockFlags::empty(); > > self.buffer.set_size(self.buffer.payload.len()); > > self.buffer.set_seq_nr(self.seq_nr); > > self.seq_nr +=3D 1; > > @@ -86,7 +83,7 @@ impl BlockedWriter { > > self.logical_end_of_media =3D true; > > } > > self.buffer_pos =3D 0; > > - self.bytes_written +=3D BlockHeader::SIZE; > > + self.bytes_written +=3D TapeBlock::SIZE; > > } else { > > self.buffer_pos +=3D bytes; > > } > > @@ -116,14 +113,14 @@ impl TapeWrite for BlockedWriter { > > /// END_OF_STREAM flag. > > fn finish(&mut self, incomplete: bool) -> Result { > > vec::clear(&mut self.buffer.payload[self.buffer_pos..]); > > - self.buffer.flags =3D BlockHeaderFlags::END_OF_STREAM; > > + self.buffer.flags =3D TapeBlockFlags::END_OF_STREAM; > > if incomplete { > > - self.buffer.flags |=3D BlockHeaderFlags::INCOMPLETE; > > + self.buffer.flags |=3D TapeBlockFlags::INCOMPLETE; > > } > > self.buffer.set_size(self.buffer_pos); > > self.buffer.set_seq_nr(self.seq_nr); > > self.seq_nr +=3D 1; > > - self.bytes_written +=3D BlockHeader::SIZE; > > + self.bytes_written +=3D TapeBlock::SIZE; > > let leom =3D 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 paylo= ad. > > /// > > /// 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 on= ly ever > > +/// exist as a heap-allocated value. > nit: the important part was that it is large. You can definitely have > DSTs on the stack via unsize coercsion (or alloca). Ah, I think this was meant to be a response to patch #04? Anyway, I agree; will correct this in v2. Thanks!