[pve-devel] [PATCH proxmox-ve-rs v4 05/22] frr: add common frr types

[Gabriel Goller]

On 07.07.2025 13:18, Wolfgang Bumiller wrote:
>On Wed, Jul 02, 2025 at 04:49:56PM +0200, Gabriel Goller wrote:
>> [snip]
>> +impl FromStr for FrrWord {
>> +    type Err = FrrWordError;
>> +
>> +    fn from_str(s: &str) -> Result<Self, Self::Err> {
>> +        FrrWord::new(s.to_string())
>
>^ Let's try to avoid allocating before error-checking. We could move the
>check out of `new()` into a helper, call that, then just build
>`Self(s.to_string())`.

Changed new into:

     pub fn new<T: AsRef<str> + Into<String>>(name: T)

and called new in the FromStr impl. Hope that's alright.

>> +    }
>> +}
>> +
>> +impl Display for FrrWord {
>> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
>> +        self.0.fmt(f)
>> +    }
>> +}
>> +
>> +impl AsRef<str> for FrrWord {
>> +    fn as_ref(&self) -> &str {
>> +        &self.0
>> +    }
>> +}
>> +
>> +#[derive(Error, Debug)]
>> +pub enum CommonInterfaceNameError {
>> +    #[error("interface name too long")]
>> +    TooLong,
>> +}
>> +
>> +/// Name of a interface, which is common between all protocols.
>> +///
>> +/// FRR itself doesn't enforce any limits, but the kernel does. Linux only allows interface names
>> +/// to be a maximum of 16 bytes. This is enforced by this struct.
>> +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Hash, PartialOrd, Ord)]
>> +pub struct CommonInterfaceName(String);
>> +
>> +impl FromStr for CommonInterfaceName {
>
>^ `FromStr` is not in the prelude.
>Via prelude this only provides `.parse()`, but we don't really "parse
>it".
>IMO this could also be a `new()`, and  have `TryFrom<String>` and `TryFrom<&str>`.
>We can make `fn new<T: AsRef<str> + Into<String>>(T)` to be able to
>run the check before any potential allocation...

Good point.

>> +    type Err = CommonInterfaceNameError;
>> +
>> +    fn from_str(s: &str) -> Result<Self, Self::Err> {
>> +        if s.len() <= 15 {
>> +            Ok(Self(s.to_owned()))
>> +        } else {
>> +            Err(CommonInterfaceNameError::TooLong)
>> +        }
>> +    }
>> +}
>> +
>> +impl Display for CommonInterfaceName {
>> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
>> +        self.0.fmt(f)
>> +    }
>> +}

Thanks for the review!