public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Dietmar Maurer <dietmar@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>
Subject: [pbs-devel] applied: [PATCH proxmox-backup v4 1/2] tape: changer: correctly consume data in decode_element_status_page
Date: Thu, 22 Jul 2021 11:38:54 +0200	[thread overview]
Message-ID: <356c750e-9a94-5d7a-cb49-e08686b1f156@proxmox.com> (raw)
In-Reply-To: <20210722092630.2227801-1-d.csapak@proxmox.com>

applied both patches

On 7/22/21 11:26 AM, Dominik Csapak wrote:
> instead of 'blindly' trusting the changer to deliver the fields written
> in the specification, trust the length data it returns in the header.
>
> we slice the descriptor data into equal sized chunks of the correct
> size, then we do not have care bout the len and empty checks anymore
>
> this also makes the code to read the rest of the page obsolete,
> since the next descriptor is on the correct offset anyway
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>   src/tape/changer/sg_pt_changer.rs | 31 ++-----------------------------
>   1 file changed, 2 insertions(+), 29 deletions(-)
>
> diff --git a/src/tape/changer/sg_pt_changer.rs b/src/tape/changer/sg_pt_changer.rs
> index a15d8192..098c10d8 100644
> --- a/src/tape/changer/sg_pt_changer.rs
> +++ b/src/tape/changer/sg_pt_changer.rs
> @@ -651,17 +651,9 @@ fn decode_element_status_page(
>               }
>   
>               let descr_data = reader.read_exact_allocated(len)?;
> -            let mut reader = &descr_data[..];
>   
> -            loop {
> -                if reader.is_empty() {
> -                    break;
> -                }
> -                if reader.len() < (subhead.descriptor_length as usize) {
> -                    break;
> -                }
> -
> -                let len_before = reader.len();
> +            for descriptor in descr_data.chunks_exact(subhead.descriptor_length as usize) {
> +                let mut reader = &descriptor[..];
>   
>                   match subhead.element_type_code {
>                       1 => {
> @@ -672,9 +664,6 @@ fn decode_element_status_page(
>   
>                           subhead.skip_alternate_volume_tag(&mut reader)?;
>   
> -                        let mut reserved = [0u8; 4];
> -                        reader.read_exact(&mut reserved)?;
> -
>                           result.last_element_address = Some(desc.element_address);
>   
>                           let status = TransportElementStatus {
> @@ -691,9 +680,6 @@ fn decode_element_status_page(
>   
>                           subhead.skip_alternate_volume_tag(&mut reader)?;
>   
> -                        let mut reserved = [0u8; 4];
> -                        reader.read_exact(&mut reserved)?;
> -
>                           result.last_element_address = Some(desc.element_address);
>   
>                           if subhead.element_type_code == 3 {
> @@ -763,19 +749,6 @@ fn decode_element_status_page(
>                       }
>                       code => bail!("got unknown element type code {}", code),
>                   }
> -
> -                // we have to consume the whole descriptor size, else
> -                // our position in the reader is not correct
> -                let len_after = reader.len();
> -                let have_read = len_before - len_after;
> -                let desc_len = subhead.descriptor_length as usize;
> -                if desc_len > have_read {
> -                    let mut left_to_read = desc_len - have_read;
> -                    if left_to_read > len_after {
> -                        left_to_read = len_after; // reader has not enough data?
> -                    }
> -                    let _ = reader.read_exact_allocated(left_to_read)?;
> -                }
>               }
>           }
>   




      parent reply	other threads:[~2021-07-22  9:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22  9:26 [pbs-devel] " Dominik Csapak
2021-07-22  9:26 ` [pbs-devel] [PATCH proxmox-backup v4 2/2] tape: changer: improve error message on wrong counts Dominik Csapak
2021-07-22  9:38 ` Dietmar Maurer [this message]

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=356c750e-9a94-5d7a-cb49-e08686b1f156@proxmox.com \
    --to=dietmar@proxmox.com \
    --cc=d.csapak@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