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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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.

> 
> 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.

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