Commit 1cdcf8f3 authored by Iustin Pop's avatar Iustin Pop
Browse files

Rework unit parsing



Due to how conversions were implemented previously, 1TB failed to
parse on 32-bit, as we were overflowing during computation, even
though the final result would fit easily.

This patch moves the parsing of the scaling factor to a separate
function, and all the conversions are done via the Rational type
(which has unlimited arbitrary precision), and conversion to the
desired type only happens at the last step.

The unit-tests are adjusted too, unfortunately they use the same
algorithm as the code… suggestions on how to improve things are
welcome.
Signed-off-by: default avatarIustin Pop <iustin@google.com>
Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
parent 1bf72492
......@@ -537,17 +537,20 @@ prop_Utils_select_undefv lst1 (NonEmpty lst2) =
cndlist = flist ++ tlist ++ [undefined]
prop_Utils_parseUnit (NonNegative n) =
Utils.parseUnit (show n) == Types.Ok n &&
Utils.parseUnit (show n ++ "m") == Types.Ok n &&
(case Utils.parseUnit (show n ++ "M") of
Types.Ok m -> if n > 0
then m < n -- for positive values, X MB is < than X MiB
else m == 0 -- but for 0, 0 MB == 0 MiB
Types.Bad _ -> False) &&
Utils.parseUnit (show n ++ "g") == Types.Ok (n*1024) &&
Utils.parseUnit (show n ++ "t") == Types.Ok (n*1048576) &&
Types.isBad (Utils.parseUnit (show n ++ "x")::Types.Result Int)
where _types = n::Int
Utils.parseUnit (show n) ==? Types.Ok n .&&.
Utils.parseUnit (show n ++ "m") ==? Types.Ok n .&&.
Utils.parseUnit (show n ++ "M") ==? Types.Ok (truncate n_mb::Int) .&&.
Utils.parseUnit (show n ++ "g") ==? Types.Ok (n*1024) .&&.
Utils.parseUnit (show n ++ "G") ==? Types.Ok (truncate n_gb::Int) .&&.
Utils.parseUnit (show n ++ "t") ==? Types.Ok (n*1048576) .&&.
Utils.parseUnit (show n ++ "T") ==? Types.Ok (truncate n_tb::Int) .&&.
printTestCase "Internal error/overflow?"
(n_mb >=0 && n_gb >= 0 && n_tb >= 0) .&&.
property (Types.isBad (Utils.parseUnit (show n ++ "x")::Types.Result Int))
where _types = (n::Int)
n_mb = (fromIntegral n::Rational) * 1000 * 1000 / 1024 / 1024
n_gb = n_mb * 1000
n_tb = n_gb * 1000
-- | Test list for the Utils module.
testSuite "Utils"
......
......@@ -40,6 +40,7 @@ module Ganeti.HTools.Utils
import Data.Char (toUpper)
import Data.List
import Data.Ratio ((%))
import Debug.Trace
......@@ -163,6 +164,25 @@ printTable lp header rows isnum =
unlines . map ((++) lp) . map ((:) ' ' . unwords) $
formatTable (header:rows) isnum
-- | Converts a unit (e.g. m or GB) into a scaling factor.
parseUnitValue :: (Monad m) => String -> m Rational
parseUnitValue unit
-- binary conversions first
| null unit = return 1
| unit == "m" || upper == "MIB" = return 1
| unit == "g" || upper == "GIB" = return kbBinary
| unit == "t" || upper == "TIB" = return $ kbBinary * kbBinary
-- SI conversions
| unit == "M" || upper == "MB" = return mbFactor
| unit == "G" || upper == "GB" = return $ mbFactor * kbDecimal
| unit == "T" || upper == "TB" = return $ mbFactor * kbDecimal * kbDecimal
| otherwise = fail $ "Unknown unit '" ++ unit ++ "'"
where upper = map toUpper unit
kbBinary = 1024
kbDecimal = 1000
decToBin = kbDecimal / kbBinary -- factor for 1K conversion
mbFactor = decToBin * decToBin -- twice the factor for just 1K
-- | Tries to extract number and scale from the given string.
--
-- Input must be in the format NUMBER+ SPACE* [UNIT]. If no unit is
......@@ -172,20 +192,10 @@ parseUnit :: (Monad m, Integral a, Read a) => String -> m a
parseUnit str =
-- TODO: enhance this by splitting the unit parsing code out and
-- accepting floating-point numbers
case reads str of
case (reads str::[(Int, String)]) of
[(v, suffix)] ->
let unit = dropWhile (== ' ') suffix
upper = map toUpper unit
siConvert x = x * 1000000 `div` 1048576
in case () of
_ | null unit -> return v
| unit == "m" || upper == "MIB" -> return v
| unit == "M" || upper == "MB" -> return $ siConvert v
| unit == "g" || upper == "GIB" -> return $ v * 1024
| unit == "G" || upper == "GB" -> return $ siConvert
(v * 1000)
| unit == "t" || upper == "TIB" -> return $ v * 1048576
| unit == "T" || upper == "TB" -> return $
siConvert (v * 1000000)
| otherwise -> fail $ "Unknown unit '" ++ unit ++ "'"
in do
scaling <- parseUnitValue unit
return $ truncate (fromIntegral v * scaling)
_ -> fail $ "Can't parse string '" ++ str ++ "'"
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment