Re: [pve-devel] [PATCH pve-installer 5/6] common: add more descriptive errors for invalid network configs

["Christoph Heiss" <c.heiss@proxmox.com>]

On Tue Apr 22, 2025 at 6:27 PM CEST, Michael Köppl wrote:
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
>  proxmox-installer-common/src/utils.rs | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs
> index 8adcec0..49f1c9f 100644
> --- a/proxmox-installer-common/src/utils.rs
> +++ b/proxmox-installer-common/src/utils.rs
> @@ -18,6 +18,20 @@ pub enum CidrAddressParseError {
>      InvalidMask(Option<ParseIntError>),
>  }
>
> +impl fmt::Display for CidrAddressParseError {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        let msg = match &self {
> +            CidrAddressParseError::NoDelimiter => {
> +                String::from("No delimiter for separating address and mask was found")
> +            }
> +            CidrAddressParseError::InvalidAddr(addr_parse_error) => format!("{addr_parse_error}"),
> +            CidrAddressParseError::InvalidMask(parse_int_error) => format!("{:?}", parse_int_error),
> +        };
> +
> +        write!(f, "Invalid CIDR: {msg}")
> +    }

This can be rewritten slightly to avoid unneeded string allocation and
formatting:

fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
    write!(f, "Invalid CIDR: ")?;

    match self {
	CidrAddressParseError::NoDelimiter => {
	    write!(f, "No delimiter for separating address and mask was found")
	}
	CidrAddressParseError::InvalidAddr(err) => write!(f, "{err}"),
	CidrAddressParseError::InvalidMask(err) => {
	    if let Some(e) = err {
		write!(f, "{e:?}")
	    } else {
		write!(f, "Invalid mask")
	    }
	}
    }
}

> +}
> +
>  /// An IP address (IPv4 or IPv6), including network mask.
>  ///
>  /// See the [`IpAddr`] type for more information how IP addresses are handled.
> @@ -110,7 +124,7 @@ impl<'de> Deserialize<'de> for CidrAddress {
>      {
>          let s: String = Deserialize::deserialize(deserializer)?;
>          s.parse()
> -            .map_err(|_| serde::de::Error::custom("invalid CIDR"))
> +            .map_err(|err| serde::de::Error::custom(format!("{err}")))

nit: Could also just use

    s.parse().map_err(serde::de::Error::custom)

here now.

>      }
>  }
>



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel