From 96859959bd24d99f51d5dea3b5ec7bc4bf7071f3 Mon Sep 17 00:00:00 2001 From: Damien Lespiau Date: Tue, 6 Oct 2009 12:02:15 +0100 Subject: [PATCH] [units] Be more strict in the grammar we are parsing Current parsing of units has a number of shortcomings: * a number followed by trailing space (without any unit specified) was not recognized, * "5 emeralds" was parsed as 5em, * the way we parse the digits after the separator makes us lose precision for no good reason (5.0 is parsed as 5.00010014...f which makes g_assert_cmpfloat() fail) Let's define a stricter grammar we can recognize and try to do so. The description is in EBNF form, removing the optional <> which is a pain when having to write DocBook, and using '' for the terminal symbols. Last step, add more ClutterUnits unit test to get a better coverage of the grammar we want to parse. Reviewed-by: Emmanuele Bassi --- clutter/clutter-units.c | 76 +++++++++++++++++++----------- tests/conform/test-clutter-units.c | 22 +++++++++ 2 files changed, 70 insertions(+), 28 deletions(-) diff --git a/clutter/clutter-units.c b/clutter/clutter-units.c index 788bab917..069bed0bc 100644 --- a/clutter/clutter-units.c +++ b/clutter/clutter-units.c @@ -362,10 +362,14 @@ clutter_units_to_pixels (ClutterUnits *units) * A #ClutterUnits expressed in string should match: * * |[ - * number: [0-9] - * unit_value: <number>+ - * unit_name: px|pt|mm|em - * units: <unit_value> <unit_name> + * units: wsp* unit-value wsp* unit-name? wsp* + * unit-value: number + * unit-name: 'px' | 'pt' | 'mm' | 'em' + * number: digit+ + * | digit* sep digit+ + * sep: '.' | ',' + * digit: '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' + * wsp: (#0x20 | #0x9 | #0xA | #0xB | #0xC | #0xD)+ * ]| * * For instance, these are valid strings: @@ -375,6 +379,7 @@ clutter_units_to_pixels (ClutterUnits *units) * 5.1 em * 24 pt * 12.6 mm + * .3 cm * ]| * * While these are not: @@ -401,51 +406,66 @@ clutter_units_from_string (ClutterUnits *units, g_return_val_if_fail (units != NULL, FALSE); g_return_val_if_fail (str != NULL, FALSE); - /* Ensure that the first character is a digit */ + /* strip leading space */ while (g_ascii_isspace (*str)) str++; if (*str == '\0') return FALSE; - if (!g_ascii_isdigit (*str)) - return FALSE; - /* integer part */ value = (gfloat) strtoul (str, (char **) &str, 10); if (*str == '.' || *str == ',') { - glong frac = 100000; + gfloat divisor = 0.1; - while (g_ascii_isdigit (*++str)) + /* 5.cm is not a valid number */ + if (!g_ascii_isdigit (*++str)) + return FALSE; + + while (g_ascii_isdigit (*str)) { - frac += (*str - '0') * frac; - frac /= 10; + value += (*str - '0') * divisor; + divisor *= 0.1; + str++; } - - value += (1.0f / (gfloat) frac); } + while (g_ascii_isspace (*str)) + str++; + /* assume pixels by default, if no unit is specified */ if (*str == '\0') unit_type = CLUTTER_UNIT_PIXEL; - else + else if (strncmp (str, "em", 2) == 0) { - while (g_ascii_isspace (*str)) - str++; - - if (strncmp (str, "em", 2) == 0) - unit_type = CLUTTER_UNIT_EM; - else if (strncmp (str, "mm", 2) == 0) - unit_type = CLUTTER_UNIT_MM; - else if (strncmp (str, "pt", 2) == 0) - unit_type = CLUTTER_UNIT_POINT; - else if (strncmp (str, "px", 2) == 0) - unit_type = CLUTTER_UNIT_PIXEL; - else - return FALSE; + unit_type = CLUTTER_UNIT_EM; + str += 2; } + else if (strncmp (str, "mm", 2) == 0) + { + unit_type = CLUTTER_UNIT_MM; + str += 2; + } + else if (strncmp (str, "pt", 2) == 0) + { + unit_type = CLUTTER_UNIT_POINT; + str += 2; + } + else if (strncmp (str, "px", 2) == 0) + { + unit_type = CLUTTER_UNIT_PIXEL; + str += 2; + } + else + return FALSE; + + /* ensure the unit is only followed by white space */ + while (g_ascii_isspace (*str)) + str++; + if (*str != '\0') + return FALSE; units->unit_type = unit_type; units->value = value; diff --git a/tests/conform/test-clutter-units.c b/tests/conform/test-clutter-units.c index 2a8f8ba13..3506eaf10 100644 --- a/tests/conform/test-clutter-units.c +++ b/tests/conform/test-clutter-units.c @@ -31,14 +31,36 @@ test_units_string (TestConformSimpleFixture *fixture, g_assert (clutter_units_get_unit_type (&units) == CLUTTER_UNIT_PIXEL); g_assert_cmpfloat (clutter_units_get_unit_value (&units), ==, 10); + g_assert (clutter_units_from_string (&units, "10 ") == TRUE); + g_assert (clutter_units_get_unit_type (&units) == CLUTTER_UNIT_PIXEL); + g_assert_cmpfloat (clutter_units_get_unit_value (&units), ==, 10); + g_assert (clutter_units_from_string (&units, "5 em") == TRUE); g_assert (clutter_units_get_unit_type (&units) == CLUTTER_UNIT_EM); g_assert_cmpfloat (clutter_units_get_unit_value (&units), ==, 5); + g_assert (clutter_units_from_string (&units, "5 emeralds") == FALSE); + g_assert (clutter_units_from_string (&units, " 16 mm") == TRUE); g_assert (clutter_units_get_unit_type (&units) == CLUTTER_UNIT_MM); g_assert_cmpfloat (clutter_units_get_unit_value (&units), ==, 16); + g_assert (clutter_units_from_string (&units, " 24 pt ") == TRUE); + g_assert (clutter_units_get_unit_type (&units) == CLUTTER_UNIT_POINT); + g_assert_cmpfloat (clutter_units_get_unit_value (&units), ==, 24); + + g_assert (clutter_units_from_string (&units, " 32 em garbage") == FALSE); + + g_assert (clutter_units_from_string (&units, "5.1mm") == TRUE); + g_assert (clutter_units_get_unit_type (&units) == CLUTTER_UNIT_MM); + g_assert_cmpfloat (clutter_units_get_unit_value (&units), ==, 5.1f); + + g_assert (clutter_units_from_string (&units, "5,mm") == FALSE); + + g_assert (clutter_units_from_string (&units, ".5pt") == TRUE); + g_assert (clutter_units_get_unit_type (&units) == CLUTTER_UNIT_POINT); + g_assert_cmpfloat (clutter_units_get_unit_value (&units), ==, 0.5f); + g_assert (clutter_units_from_string (&units, "1 pony") == FALSE); clutter_units_from_pt (&units, 24.0);