Re: [pbs-devel] [PATCH proxmox-backup 2/2] node: status: declutter kernel-version

[Gabriel Goller <g.goller@proxmox.com>]

On 11/23/23 17:29, Thomas Lamprecht wrote:

> Am 23/11/2023 um 17:15 schrieb Dietmar Maurer:
>>> +    let kversion = KernelVersionInformation {
>>> +        sysname: uname.sysname().to_os_string().into_string().unwrap(),
>>> +        release: uname.release().to_os_string().into_string().unwrap(),
>>> +        version: uname.version().to_os_string().into_string().unwrap(),
>>> +        machine: uname.machine().to_os_string().into_string().unwrap(),
>>> +    };
>> Is it really safe to use unwrap here? If so, I normally add a short
>> comment to the code to explain why.
>>
> Yeah, I *really* dislike .unwrap() calls, especially if in common code,
> i.e., not isolated into it's own function with a clear comment that
> states why this never can fail, and IMO it can fail because those strings
> are just bytes for the kernel and can be set on boot, so it might be
> possible to set them to something that isn't valid unicode..
>
> So, please use unwarp_or and maybe it would be better to move this to
> an impl on the KernelVersionInformation struct, that could also host
> a to_legacy() printing the old unwieldy string, as I'd not just switch
> the old variable from string to object, that's an api break.
Agree, I will use a `unwrap_or` and return an empty string.
Didn't know backwards compat was a thing here, so I will also return the
old property with the legacy string :)