On 4/15/20 2:41 PM, Dominic Jäger wrote:
> 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].
>
albeit I'm not to sure if the unicode stuff is really true, I tried:
perl -we '"Ⅲ" =~ /\d/ or die "no match"'
where "Ⅲ" is the roman numeral three (U+2162) and that does not matches,
but it could be wrong encoding or whatever...
>>> + 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)
> ?
we often do:
next if !defined($dev);
at least if we expect that $dev can be really undef here..
>>
>> or, if you want to make it shorter with something like:
>>
>> my $dev;
>> my $dev_sizes = { eval { map { $dev = $_ // ''; $_ => get_blockdev_size($_)
>> } @$devices } };
no use setting $dev if only $_ is used afterwards ;)
>> 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.
for this I'd just go for the `for my $..` loop, IMO the map isn't worth the
hassle here.
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
