Re: [pve-devel] [PATCH pve-common] cli: fix #6762: only copy properties if defined

[Nicolas Frey <n.frey@proxmox.com>]

On 10/7/25 4:46 PM, Thomas Lamprecht wrote:
> Am 07.10.25 um 15:56 schrieb Nicolas Frey:
>> Adds a defined check to the copy, as to not result in the bugfixes
>> reported error when double tabbing on `pveceph status`.
> Feel encouraged to include errors in verbatim in commit message,
> while we own bugzilla.proxmox.com and thus it's unlikely that it
> will be gone without notice or migration it's still always nice to
> keep git commits self-sufficient w.r.t core information. Makes it
> also easier to find a commit if one just searches for the error
> message.
> 

OK, will do in the future. Thanks for letting me know!

>>
>> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6762
>> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
>> ---
>>  src/PVE/CLIHandler.pm | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
>> index 89cb7b7..cdd71c7 100644
>> --- a/src/PVE/CLIHandler.pm
>> +++ b/src/PVE/CLIHandler.pm
>> @@ -455,7 +455,8 @@ my $print_bash_completion = sub {
>>  
>>      my $info = $class->map_method_by_name($name);
>>  
>> -    my $prop = { %{ $info->{parameters}->{properties} } }; # copy
>> +    my $prop = { %{ $info->{parameters}->{properties} } }
>> +        if defined $info->{parameters}->{properties}; # copy
> 
> You cannot know without having lots of perl experience, or having had
> the bad luck of debugging this yourself early on, but a conditional
> declaration of a variable is Real Bad™ perl, as it will retain the
> value of the last time the condition held true for the case it's false
> now.
> 

TIL another weird quirk about perl. Thanks for pointing it out and
saving me from needing to debug this!

Will send a v2 momentarily.

> See the NOTE at the end of the "Statement Modifiers" [0] section for
> details.
> 
> [0]: https://perldoc.perl.org/perlsyn#Statement-Modifiers
> 
> Safe alternatives are:
> 
> my $prop;
> $prop = { $info->{parameters}->{properties}->%* } if defined($info->{parameters}->{properties});
> 
> Similar, but with normal if block, which can be combined with adding a
> intermediate variable.
> 
> my $prop;
> if (defined(my $properties = $info->{parameters}->{properties})) {
>     $prop = { $properties->%* }; # clone
> }
> 
> Alternatively, one can use a intermediate variable + a fallback value
> 
> my $parameter_properties = $info->{parameters}->{properties} // {};
> my $prop = { $parameter_properties->%* }; # copy
> 
> Alternatively, one can use a ternary statement:
> 
> $prop = defined($info->{parameters}->{properties})
>  ? { $info->{parameters}->{properties}->%* }
>  : {};
> 
> The last one is IMO a bit long here.
> 
> 
>>      $prop = { %$prop, %$formatter_properties } if $formatter_properties;
>>  
>>      my $print_parameter_completion = sub {
> 



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