Thank you for taking a look!
Most makes sense to me and is already changed.
On Wed, Apr 15, 2020 at 11:28:34AM +0200, Fabian Grünbichler wrote:
>
> > + if ($max_amount !~ /^[0-9]+$/);
>
> \d instead of [0-9]
This was \d previously. However, it was recommended to me to change it to [0-9]
in v2 because of Unicode difficulties IIRC [0].
> > + foreach my $dev (@$devices) {
> > + my $device_size = get_blockdev_size($dev);
>
> add an eval here, and set $dev_sizes->{$dev} directly
>
> > + die "Not wiping anything because could not get size of $dev." if
> > !defined($device_size);
>
> and add ' - $@' to the message here. but note - you probably want to
> check $dev for definedness at the start of the loop, else this triggers
> a warning for printing undef ;)
"$dev for definedness at the start of the loop" like
foreach my $dev (@$devices) {
if !defined($dev)
?
>
> or, if you want to make it shorter with something like:
>
> my $dev;
> my $dev_sizes = { eval { map { $dev = $_ // ''; $_ => get_blockdev_size($_) }
> @$devices } };
> die "... $dev ... - $@\n" if $@;
>
> (could be made shorter if we don't care about including $dev in the
> error message, or make get_blockdev_size include it).
I'd have fun writing that but I can't see how the amount of reduced lines
justifies the IMO increased complexity yet.
[0] https://pve.proxmox.com/pipermail/pve-devel/2020-March/042078.html
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
