applied On Fri, Dec 15, 2017 at 10:58:10AM +0100, Dominik Csapak wrote: > converting from 0.5 gb to mb resulted in 0 mb > with this patch it correctly returns 512 > > also add tests and catch more errors > > Signed-off-by: Dominik Csapak <d.csa...@proxmox.com> > --- > changes from v2: > * unified up and down converting (with regards to ceil/floor) > * disallow empty/non-defined and '.' as value (has no real value) > * catch undefined to/from > * catch undefined variables i test > * added test cases for ceil/floor when converting to bigger unit > * added content of value to error message > * made condition for to/from check more readable > * added comment giving examples for allowed format of value > src/PVE/Tools.pm | 34 +++++++++++++++++---------- > test/Makefile | 1 + > test/convert_size_test.pl | 60 > +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 82 insertions(+), 13 deletions(-) > create mode 100755 test/convert_size_test.pl > > diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm > index 6d579d8..f43424b 100644 > --- a/src/PVE/Tools.pm > +++ b/src/PVE/Tools.pm > @@ -1629,9 +1629,16 @@ sub encrypt_pw { > } > > # intended usage: convert_size($val, "kb" => "gb") > -# on reduction (converting to a bigger unit) we round up by default if > -# information got lost. E.g. `convert_size(1023, "b" => "kb")` returns 1 > +# we round up to the next integer by default > +# E.g. `convert_size(1023, "b" => "kb")` returns 1 > # use $no_round_up to switch this off, above example would then return 0 > +# this is also true for converting down e.g. 0.0005 gb to mb returns 1 > +# (0 if $no_round_up is true) > +# allowed formats for value: > +# 1234 > +# 1234. > +# 1234.1234 > +# .1234 > sub convert_size { > my ($value, $from, $to, $no_round_up) = @_; > > @@ -1644,21 +1651,22 @@ sub convert_size { > pb => 5, > }; > > - $from = lc($from); $to = lc($to); > + die "no value given" > + if !defined($value) || $value eq ""; > + > + $from = lc($from // ''); $to = lc($to // ''); > die "unknown 'from' and/or 'to' units ($from => $to)" > - if !(defined($units->{$from}) && defined($units->{$to})); > + if !defined($units->{$from}) || !defined($units->{$to}); > > - my $shift_amount = $units->{$from} - $units->{$to}; > + die "value '$value' is not a valid, positive number" > + if $value !~ m/^(?:[0-9]+\.?[0-9]*|[0-9]*\.[0-9]+)$/; > > - if ($shift_amount > 0) { > - $value <<= ($shift_amount * 10); > - } elsif ($shift_amount < 0) { > - my $remainder = ($value & (1 << abs($shift_amount)*10) - 1); > - $value >>= abs($shift_amount) * 10; > - $value++ if $remainder && !$no_round_up; > - } > + my $shift_amount = ($units->{$from} - $units->{$to}) * 10; > + > + $value *= 2**$shift_amount; > + $value++ if !$no_round_up && ($value - int($value)) > 0.0; > > - return $value; > + return int($value); > } > > 1; > diff --git a/test/Makefile b/test/Makefile > index 894093b..b6fe6e0 100644 > --- a/test/Makefile > +++ b/test/Makefile > @@ -8,6 +8,7 @@ check: > for d in $(SUBDIRS); do $(MAKE) -C $$d check; done > ./lock_file.pl > ./calendar_event_test.pl > + ./convert_size_test.pl > > install: check > distclean: clean > diff --git a/test/convert_size_test.pl b/test/convert_size_test.pl > new file mode 100755 > index 0000000..93e88b1 > --- /dev/null > +++ b/test/convert_size_test.pl > @@ -0,0 +1,60 @@ > +#!/usr/bin/perl > + > +use lib '../src'; > +use strict; > +use warnings; > +use Data::Dumper; > +use Test::More; > + > +use PVE::Tools; > + > +my $tests = [ > + [ > + 1, # input value > + 'gb', # from > + 'kb', # to > + undef, # no_round_up > + 1*1024*1024, # result > + undef, # error string > + ], > + [ -1, 'gb', 'kb', undef, 1*1024*1024, "value '-1' is not a valid, > positive number" ], > + [ 1.5, 'gb', 'kb', undef, 1.5*1024*1024 ], > + [ 0.0005, 'gb', 'mb', undef, 1 ], > + [ 0.0005, 'gb', 'mb', 1, 0 ], > + [ '.5', 'gb', 'kb', undef, .5*1024*1024 ], > + [ '1.', 'gb', 'kb', undef, 1.*1024*1024 ], > + [ 0.5, 'mb', 'gb', undef, 1, ], > + [ 0.5, 'mb', 'gb', 1, 0, ], > + [ '.', 'gb', 'kb', undef, 0, "value '.' is not a valid, positive number" > ], > + [ '', 'gb', 'kb', undef, 0, "no value given" ], > + [ '1.1.', 'gb', 'kb', undef, 0, "value '1.1.' is not a valid, positive > number" ], > + [ 500, 'kb', 'kb', undef, 500, ], > + [ 500000, 'b', 'kb', undef, 489, ], > + [ 500000, 'b', 'kb', 0, 489, ], > + [ 500000, 'b', 'kb', 1, 488, ], > + [ 128*1024 - 1, 'b', 'kb', 0, 128, ], > + [ 128*1024 - 1, 'b', 'kb', 1, 127, ], > + [ "abcdef", 'b', 'kb', 0, 0, "value 'abcdef' is not a valid, positive > number" ], > + [ undef, 'b', 'kb', 0, 0, "no value given" ], > + [ 0, 'b', 'pb', 0, 0, ], > + [ 0, 'b', 'yb', 0, 0, "unknown 'from' and/or 'to' units (b => yb)"], > + [ 0, 'b', undef, 0, 0, "unknown 'from' and/or 'to' units (b => )"], > +]; > + > +foreach my $test (@$tests) { > + my ($input, $from, $to, $no_round_up, $expect, $error) = @$test; > + > + my $result = eval { PVE::Tools::convert_size($input, $from, $to, > $no_round_up); }; > + my $err = $@; > + $input = $input // ""; > + $from = $from // ""; > + $to = $to // ""; > + if ($error) { > + like($err, qr/^\Q$error\E/, "expected error for $input $from -> $to: > $error"); > + } else { > + my $round = $no_round_up ? 'floor' : 'ceil'; > + is($result, $expect, "$input $from converts to $expect $to ($round)"); > + } > +}; > + > +done_testing(); > -- > 2.11.0
_______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel