[master 4/4] Fix a bug in reading a size specification with a radix in the numeric part.

mulkieran installerbot-noreply at redhat.com
Fri May 22 13:59:35 UTC 2015


From: mulhern <amulhern at redhat.com>

Since the | operator is not greedy, the match would return the first of the
three base alternatives that worked, and would split, e.g., "0.0" into
"0" and ".0" instead of "0.0" and nothing.

Now, base is a single alternative, that should capture numbers with a
decimal point correctly.

Fixes a bug introduced in commit 65703bfc851f6d919f8e66e1a635ace5835ea6c1.

Other changes are:
* No longer allows spaces between optional sign and number. Decimal
constructor will choke on '- 2', so might as well not accept it in regular
expression.
* Uses verbose regular expression, to make it a little easier to read.
Uses separate compile before match call for the same reason.
* No longer allows trailing non-whitespace characters after a separating
space, e.g., "1 KiB or maybe 2". Still  allows trailing or leading
whitespace characters. Never allowed leading non-whitespace characters,
e.g., "nearly 1 PiB".

Other non-changes are:
* Still allows juxtaposition of value and spec, e.g., "1.0e+1KiB".
* Still allows leading 0s in a number, even if they are pointless, e.g., "009".

Adds a few more tests.

Signed-off-by: mulhern <amulhern at redhat.com>
---
 blivet/size.py     | 12 +++++++++++-
 tests/size_test.py | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/blivet/size.py b/blivet/size.py
index ff9e8cb..10fa348 100644
--- a/blivet/size.py
+++ b/blivet/size.py
@@ -178,7 +178,17 @@ def parseSpec(spec):
     if radix != '.':
         spec = spec.replace(radix, '.')
 
-    m = re.match(r'(?P<numeric>(-|\+)?\s*(?P<base>([0-9]+)|([0-9]*\.[0-9]+)|([0-9]+\.[0-9]*))(?P<exp>(e|E)(-|\+)[0-9]+)?)\s*(?P<rest>[^\s]*)$', spec.strip())
+    spec_re = re.compile(
+       r"""(?P<numeric> # the numeric part consists of three parts, below
+           (-|\+)? # optional sign character
+           (?P<base>([0-9]*(\.[0-9]|[0-9]\.|[0-9])[0-9]*)) # the base
+           (?P<exp>(e|E)(-|\+)[0-9]+)?) # optional exponent
+           \s* # whitespace
+           (?P<rest>[^\s]*$) # the units specification
+        """,
+        re.VERBOSE
+    )
+    m = re.match(spec_re, spec.strip())
     if not m:
         raise ValueError("invalid size specification", spec)
 
diff --git a/tests/size_test.py b/tests/size_test.py
index 9416d71..7c12624 100644
--- a/tests/size_test.py
+++ b/tests/size_test.py
@@ -200,6 +200,57 @@ def testScientificNotation(self):
         self.assertEqual(size.parseSpec("1E-4KB"), Decimal("0.1"))
         self.assertEqual(Size("1E-10KB"), Size(0))
 
+        with self.assertRaises(ValueError):
+            # this is an exponent w/out a base
+            size.parseSpec("e+0")
+
+    def testFloatingPointStr(self):
+        self.assertEqual(size.parseSpec("1.5e+0 KiB"), Decimal(1536))
+        self.assertEqual(size.parseSpec("0.0"), Decimal(0))
+        self.assertEqual(size.parseSpec("0.9 KiB"), Decimal("921.6"))
+        self.assertEqual(size.parseSpec("1.5 KiB"), Decimal(1536))
+        self.assertEqual(size.parseSpec("0.5 KiB"), Decimal(512))
+        self.assertEqual(size.parseSpec(".5 KiB"), Decimal(512))
+        self.assertEqual(size.parseSpec("1. KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec("-1. KiB"), Decimal(-1024))
+        self.assertEqual(size.parseSpec("+1. KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec("+1.0000000e+0 KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec("+.5 KiB"), Decimal(512))
+
+        with self.assertRaises(ValueError):
+            # this is a fragment of an arithmetic expression
+            size.parseSpec("+ 1 KiB")
+
+        with self.assertRaises(ValueError):
+            # this is a fragment of an arithmetic expression
+            size.parseSpec("- 1 KiB")
+
+        with self.assertRaises(ValueError):
+            # this is a lonely .
+            size.parseSpec(". KiB")
+
+        with self.assertRaises(ValueError):
+            # this has a fragmentary exponent
+            size.parseSpec("1.0e+ KiB")
+
+        with self.assertRaises(ValueError):
+            # this is a version string, not a number
+            size.parseSpec("1.0.0")
+
+    def testWhiteSpace(self):
+        self.assertEqual(size.parseSpec("1 KiB "), Decimal(1024))
+        self.assertEqual(size.parseSpec(" 1 KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec(" 1KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec(" 1e+0KiB"), Decimal(1024))
+        with self.assertRaises(ValueError):
+            size.parseSpec("1 KiB just a lot of stray characters")
+        with self.assertRaises(ValueError):
+            size.parseSpec("just 1 KiB")
+
+    def testLeadingZero(self):
+        self.assertEqual(size.parseSpec("001 KiB"), Decimal(1024))
+        self.assertEqual(size.parseSpec("1e+01"), Decimal(10))
+
     def testPickling(self):
         s = Size("10 MiB")
         self.assertEqual(s, cPickle.loads(cPickle.dumps(s)))


-- 
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/aebb1fe6c916a69de935b46d397258a9e44e93cf


More information about the anaconda-patches mailing list