From affa696027caa296e8e242f668d8076668733341 Mon Sep 17 00:00:00 2001 From: David Neto Date: Mon, 24 Aug 2015 15:33:14 -0400 Subject: [PATCH] Fix some bugs in parsing literals. - a single hyphen is a string, not a number. - a string with more than one period is a string, not a number - check for string overflow Add some unit tests --- CMakeLists.txt | 1 + source/text.cpp | 35 ++++++++++----- test/TextLiteral.cpp | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 12 deletions(-) create mode 100644 test/TextLiteral.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 8157b39..18732d1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -171,6 +171,7 @@ if (TARGET gtest) ${CMAKE_CURRENT_SOURCE_DIR}/test/OpcodeSplit.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/OperandTableGet.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/TextAdvance.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test/TextLiteral.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/TextWordGet.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/TextToBinary.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/TextDestroy.cpp diff --git a/source/text.cpp b/source/text.cpp index dd4109b..9a412aa 100644 --- a/source/text.cpp +++ b/source/text.cpp @@ -34,7 +34,7 @@ #include #include -#include +#include #include #include @@ -247,14 +247,11 @@ spv_result_t spvTextToUInt32(const char *textValue, uint32_t *pValue) { spv_result_t spvTextToLiteral(const char *textValue, spv_literal_t *pLiteral) { bool isSigned = false; - bool isFloat = false; + int numPeriods = 0; bool isString = false; - if ('-' == textValue[0]) { - isSigned = true; - } - - for (uint64_t index = 0; index < strlen(textValue); ++index) { + const size_t len = strlen(textValue); + for (uint64_t index = 0; index < len; ++index) { switch (textValue[index]) { case '0': case '1': @@ -268,19 +265,33 @@ spv_result_t spvTextToLiteral(const char *textValue, spv_literal_t *pLiteral) { case '9': break; case '.': - isFloat = true; + numPeriods++; + break; + case '-': + if (index == 0) { + isSigned = true; + } else { + isString = true; + } break; default: isString = true; + index = len; // break out of the loop too. break; } } - if (isString) { + pLiteral->type = spv_literal_type_t(99); + + if (isString || numPeriods > 1 || (isSigned && len==1)) { + // TODO(dneto): Quotes should be required, and stripped. + // TODO(dneto): Allow escaping. pLiteral->type = SPV_LITERAL_TYPE_STRING; - strncpy(pLiteral->value.str, textValue, strlen(textValue)); - } else if (isFloat) { - double d = strtod(textValue, nullptr); + // Need room for the null-terminator. + if (len + 1 > sizeof(pLiteral->value.str)) return SPV_ERROR_OUT_OF_MEMORY; + strncpy(pLiteral->value.str, textValue, len+1); + } else if (numPeriods == 1) { + double d = std::strtod(textValue, nullptr); float f = (float)d; if (d == (double)f) { pLiteral->type = SPV_LITERAL_TYPE_FLOAT_32; diff --git a/test/TextLiteral.cpp b/test/TextLiteral.cpp new file mode 100644 index 0000000..7139b43 --- /dev/null +++ b/test/TextLiteral.cpp @@ -0,0 +1,123 @@ +// Copyright (c) 2015 The Khronos Group Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and/or associated documentation files (the +// "Materials"), to deal in the Materials without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Materials, and to +// permit persons to whom the Materials are furnished to do so, subject to +// the following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Materials. +// +// MODIFICATIONS TO THIS FILE MAY MEAN IT NO LONGER ACCURATELY REFLECTS +// KHRONOS STANDARDS. THE UNMODIFIED, NORMATIVE VERSIONS OF KHRONOS +// SPECIFICATIONS AND HEADER INFORMATION ARE LOCATED AT +// https://www.khronos.org/registry/ +// +// THE MATERIALS ARE PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +// IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +// TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +// MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS. + +#include "UnitSPIRV.h" + +#include + +TEST(TextLiteral, GoodI32) { + spv_literal_t l; + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("-0", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_INT_32, l.type); + EXPECT_EQ(0, l.value.i32); + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("-2147483648", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_INT_32, l.type); + EXPECT_EQ(-2147483648, l.value.i32); +} + +TEST(TextLiteral, GoodU32) { + spv_literal_t l; + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("0", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_UINT_32, l.type); + EXPECT_EQ(0, l.value.i32); + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("4294967295", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_UINT_32, l.type); + EXPECT_EQ(4294967295, l.value.u32); +} + +TEST(TextLiteral, GoodI64) { + spv_literal_t l; + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("-2147483649", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_INT_64, l.type); + EXPECT_EQ(-2147483649, l.value.i64); +} + +TEST(TextLiteral, GoodU64) { + spv_literal_t l; + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("4294967296", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_UINT_64, l.type); + EXPECT_EQ(4294967296, l.value.u64); +} + +TEST(TextLiteral, GoodFloat) { + spv_literal_t l; + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("1.0", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_FLOAT_32, l.type); + EXPECT_EQ(1.0, l.value.f); + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("1.5", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_FLOAT_32, l.type); + EXPECT_EQ(1.5, l.value.f); + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("-.25", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_FLOAT_32, l.type); + EXPECT_EQ(-.25, l.value.f); +} + +TEST(TextLiteral, GoodString) { + spv_literal_t l; + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("-", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_STRING, l.type); + EXPECT_STREQ("-", l.value.str); + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("--", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_STRING, l.type); + EXPECT_STREQ("--", l.value.str); + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("1-2", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_STRING, l.type); + EXPECT_STREQ("1-2", l.value.str); + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("123a", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_STRING, l.type); + EXPECT_STREQ("123a", l.value.str); + + ASSERT_EQ(SPV_SUCCESS, spvTextToLiteral("12.2.3", &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_STRING, l.type); + EXPECT_STREQ("12.2.3", l.value.str); +} + +TEST(TextLiteral, StringTooLong) { + spv_literal_t l; + std::string too_long(SPV_LIMIT_LITERAL_STRING_MAX, 'a'); + EXPECT_EQ(SPV_ERROR_OUT_OF_MEMORY, spvTextToLiteral(too_long.data(), &l)); +} + +TEST(TextLiteral, GoodLongString) { + spv_literal_t l; + std::string good_long(SPV_LIMIT_LITERAL_STRING_MAX-1, 'a'); + EXPECT_EQ(SPV_SUCCESS, spvTextToLiteral(good_long.data(), &l)); + EXPECT_EQ(SPV_LITERAL_TYPE_STRING, l.type); + EXPECT_STREQ(good_long.data(), l.value.str); +} -- 2.7.4