[pve-devel] [PATCH installer 3/6] tui, ui: switch over to JSON-based protocol

[Thomas Lamprecht]

Am 06/12/2023 um 12:34 schrieb Christoph Heiss:
> Signed-off-by: Christoph Heiss <c.heiss at proxmox.com>
> ---
>  Proxmox/UI/StdIO.pm                           |  41 ++++--
>  .../src/views/install_progress.rs             | 117 ++++++++----------
>  2 files changed, 83 insertions(+), 75 deletions(-)
> 
> diff --git a/Proxmox/UI/StdIO.pm b/Proxmox/UI/StdIO.pm
> index a97245c..25d1c82 100644
> --- a/Proxmox/UI/StdIO.pm
> +++ b/Proxmox/UI/StdIO.pm
> @@ -3,10 +3,26 @@ package Proxmox::UI::StdIO;
>  use strict;
>  use warnings;
>  
> +use JSON qw(from_json to_json);
> +
>  use base qw(Proxmox::UI::Base);
>  
>  use Proxmox::Log;
>  
> +my sub send_msg : prototype($$) {

wrong prototype, the second argument isn't a scalar but a list interpreted as
hash.

That's why you had to use &send_msg on the call sites, which calls it via
reference and so does not checks prototypes at all.

Correct would be prototype($%), but that's not really catching any issues,
so it could be just left out.

In general I'd recommend to try to not overuse prototypes, they are best
for cases where one interacts with other functions or bindings that have
to map to an exact argument style, like the syscall wrapper we have in
pve-common, and even there the "is correctly called" is mostly a side-effect
(that can be circumvented as you've done here).

Prototypes basically allow you to create functions that behave like Perl
built-in functions, and e.g., take an array-ref even if a list is passed,
but for us that's seldom of use, or rather said, a bit to magic for my
taste and can lead to bugs on its own (list based to scalar prototype
results in the list-size being actually passed, way better for long term
maintenance to just do such things explicitly...

That doesn't mean that they should not be used at all, they sometimes can
have benefits, but if you need to result to use &method() to call a sub then
you're just doing something strange (and most of the time also wrong).