all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Sterz <s.sterz@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>,
	Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>,
	Thomas Lamprecht <t.lamprecht@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup 1/2] fix #3853: api: add force option to tape key change-passphrase
Date: Tue, 8 Feb 2022 16:30:08 +0100	[thread overview]
Message-ID: <23c29e64-bc74-9a8f-7b70-bad0abdb633a@proxmox.com> (raw)
In-Reply-To: <b4a5d538-72e9-2f79-8632-db6e332b217b@proxmox.com>

On 2/8/22 16:26, Dominik Csapak wrote:
> On 2/7/22 17:14, Stefan Sterz wrote:
>> On 2/7/22 16:57, Stefan Sterz wrote:
>>> On 2/7/22 15:58, Thomas Lamprecht wrote:
>>>> On 07.02.22 13:48, Stefan Sterz wrote:
> [snip]
>>>>> +    // sanity checks for "password xor --force"
>>>>> +    if force && password.is_some() {
>>>>> +        bail!("password is not allowed when using force")
>>>>> +    }
>>>>> +
>>>>> +    if !force && password.is_none() {
>>>>> +        bail!("missing parameter: password")
>>>>> +    }
>>>> Above two if's could be written slightly shorter while IMO even 
>>>> improving readability
>>>>
>>>> match (force, password) {
>>>>      (true, Some(_)) => bail!("password is not allowed when using 
>>>> force"),
>>>>      (false, None) => bail!("missing parameter: password"),
>>>>      _ => (), // OK
>>>> }
>>> This does not work, because here password is moved into the match 
>>> expression. The borrow checker will complain about it being used 
>>> later on when trying to decrypt the key configuration. You could 
>>> clone password here, but this solution strikes me as rather 
>>> "inelegant".
>
> did not look at the rest of the patch really, but i think thats wrong..
>
> couldn't you simply use the reference instead? then the value
> will not be moved?
>
> match (force, &password) {
> ...
> }
>
>
> ?
Yes sorry, using a reference works too. The value is moved when the
tuple is constructed is what I meant. This can be avoided by cloning
it or as you mentioned using a reference.




  reply	other threads:[~2022-02-08 15:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-07 12:48 Stefan Sterz
2022-02-07 12:48 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix #3853: tape cli: add force flag to " Stefan Sterz
2022-02-09 13:56   ` Wolfgang Bumiller
2022-02-07 14:58 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix #3853: api: add force option to tape " Thomas Lamprecht
2022-02-07 16:14   ` Stefan Sterz
2022-02-08 15:26     ` Dominik Csapak
2022-02-08 15:30       ` Stefan Sterz [this message]
2022-02-09 15:54         ` Thomas Lamprecht
2022-02-09 13:52 ` Wolfgang Bumiller

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=23c29e64-bc74-9a8f-7b70-bad0abdb633a@proxmox.com \
    --to=s.sterz@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=t.lamprecht@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