Re: [pve-devel] [PATCH v3 27/30] common: add deserializer for FsType

[Aaron Lauterer <a.lauterer@proxmox.com>]

On  2024-03-29  13:20, Christoph Heiss wrote:
> On Thu, Mar 28, 2024 at 02:50:25PM +0100, Aaron Lauterer wrote:
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
> [..]
>> diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
>> index 8432a2c..743e7a9 100644
>> --- a/proxmox-installer-common/src/setup.rs
>> +++ b/proxmox-installer-common/src/setup.rs
> [..]
>> @@ -471,3 +472,39 @@ where
>>
>>       serializer.collect_str(value)
>>   }
>> +
>> +pub fn deserialize_fs_type<'de, D>(deserializer: D) -> Result<FsType, D::Error>
>> +where
>> +    D: Deserializer<'de>,
>> +{
>> +    let de_fs: String = Deserialize::deserialize(deserializer)?;
>> +
>> +    let fs;
>> +    let mut raid: Option<String> = None;
>> +    let re_raid = Regex::new(r"^(?P<fs>.*) \((?P<raid>.*)\)$").map_err(de::Error::custom)?;
>> +    match re_raid.captures(de_fs.as_str()) {
>> +        Some(caps) => {
>> +            fs = caps.name("fs").unwrap().as_str().to_lowercase();
>> +            raid = Some(caps.name("raid").unwrap().as_str().to_lowercase());
>> +        },
>> +        None => fs = de_fs,
>> +    }
> 
> (nit I guess?) Instead of using a regex here (which also bloats the
> binary considerably), I'd IMO prefer a dumb `match` like in
> serialize_fstype() above.
> 
> Would be much easier to read & reason about, at least. (and probably
> less code in total as well)

Yeah, sounds reasonable. :)

> 
>> +
>> +    match fs {
>> +        t if t == "zfs" => {
>> +            let raidlevel: ZfsRaidLevel = Deserialize::deserialize(serde_json::Value::String(raid.unwrap()))
>> +                .map_err(de::Error::custom)?;
>> +            Ok(FsType::Zfs(raidlevel))
>> +        }
>> +        t if t == "btrfs" => {
>> +            let raidlevel: BtrfsRaidLevel = Deserialize::deserialize(serde_json::Value::String(raid.unwrap()))
>> +                .map_err(de::Error::custom)?;
>> +            Ok(FsType::Btrfs(raidlevel))
>> +        }
>> +        t => {
>> +            let fstype: FsType = Deserialize::deserialize(serde_json::Value::String(t))
>> +                .map_err(de::Error::custom)?;
>> +            Ok(fstype)
>> +        }
>> +    }
>> +}
>> --
>> 2.39.2
>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>