Am 30/08/2023 um 11:42 schrieb Fiona Ebner:
> Am 30.08.23 um 11:27 schrieb Thomas Lamprecht:
>> Am 23/06/2023 um 12:08 schrieb Fiona Ebner:
>>> + # filter out left-over non-existing/removed storages - avoids error on
>>> target
>>> + $state->{storeid_list} = [ grep { $storecfg->{ids}->{$_} }
>>> $state->{storeid_list}->@* ];
>>
>> looks fine in general, just wondering if we'd be better of to make
>> the grep include-condition a bit more explicit by using `exists` on
>> the hash:
>>
>> $state->{storeid_list} = [ grep { exists $storecfg->{ids}->{$_} }
>> $state->{storeid_list}->@* ];
>>
>> albeit, the value should be always truthy, so might be redundant,
>> depending on how you see this I can apply this patch or a v2.
>
> I don't like using exists() expect when it's really necessary, because
> it's way too easy picking up something that was auto-vivified
> accidentally (shouldn't happen in this case, but still).
Unwanted auto-vivification is always a bug though, and can cause problems
elsewhere, so "hiding" them more might make things harder to debug..
But yeah, if, defined would be the better choice here.
> And yes, we can
> assume the value is truthy if it's an existing storage, so I didn't
> bother with defined() either.
Yeah, while refactoring can sometimes change such invariants, I really
doubt it will happen here, so it's fine.
applied, thanks!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
