Re: [pbs-devel] [PATCH proxmox-backup 1/2] fix #3853: api: add force option to tape key change-passphrase

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 08.02.22 16:30, Stefan Sterz wrote:
> 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".

Yeah I just wrote that in the MTA from top of my head so that you get the
rough idea, using a reference here like Dominik also suggests is the obvious
way to make it work as intended.. ;-)

The whole thing, actually working, meaning it compiles ^^:

let (key, created, fingerprint) = match (force, &password) {
    (true, Some(_)) => bail!("password is not allowed when using force"),
    (false, None) => bail!("missing parameter: password"),
    (false, Some(pass)) => key_config.decrypt(&|| Ok(pass.as_bytes().to_vec()))?,
    (true, None) => {
        let (keys, _) = load_keys()?;
        let key_info = keys.get(&fingerprint).ok_or_else(|| format_err!("..."))?;
        (key_info.key, key_config.created, fingerprint)
    }
};


IMO having this as match where we know we got all cases covered has it's value and
it's not that bad to read, but I also do not want to spent to much time nitpicking
on this one, so your call if it's the whole match or just the checks in the match.