mgorny created this revision. mgorny added reviewers: labath, krytarowski, emaste, JDevlieghere. mgorny requested review of this revision.
Refactor the XML converting attribute and text getters to use LLVM API. While at it, remove some redundant error and missing XML support handling, as the called base functions do that anyway. Add tests for these methods. Note that this patch changes the getter behavior to be IMHO more correct. In particular: - negative and overflowing integers are now reported as failures to convert, rather than being wrapped over or capped - digits followed by text are now reported as failures to convert to double, rather than their numeric part being converted https://reviews.llvm.org/D110410 Files: lldb/source/Host/common/XML.cpp lldb/unittests/Host/CMakeLists.txt lldb/unittests/Host/XMLTest.cpp
Index: lldb/unittests/Host/XMLTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Host/XMLTest.cpp @@ -0,0 +1,119 @@ +//===-- XMLTest.cpp -------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Host/XML.h" +#include "gtest/gtest.h" + +using namespace lldb_private; + +#if LLDB_ENABLE_LIBXML2 + +static void assertGetElement(XMLNode &root, const char *element_name, + bool expected_uint_success, uint64_t expected_uint, + bool expected_double_success, + double expected_double) { + XMLNode node = root.FindFirstChildElementWithName(element_name); + ASSERT_TRUE(node.IsValid()); + + uint64_t uint_val; + EXPECT_EQ(node.GetElementTextAsUnsigned(uint_val, 66, 0), + expected_uint_success); + EXPECT_EQ(uint_val, expected_uint); + + double double_val; + EXPECT_EQ(node.GetElementTextAsFloat(double_val, 66.0), + expected_double_success); + EXPECT_EQ(double_val, expected_double); + + XMLNode attr_node = root.FindFirstChildElementWithName("attr"); + ASSERT_TRUE(node.IsValid()); + + EXPECT_EQ( + attr_node.GetAttributeValueAsUnsigned(element_name, uint_val, 66, 0), + expected_uint_success); + EXPECT_EQ(uint_val, expected_uint); +} + +#define ASSERT_GET(element_name, ...) \ + { \ + SCOPED_TRACE("at element/attribute " element_name); \ + assertGetElement(root, element_name, __VA_ARGS__); \ + } + +TEST(XML, GetAs) { + std::string test_xml = + "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" + "<test>\n" + " <empty/>\n" + " <text>123foo</text>\n" + " <positive-int>11</positive-int>\n" + " <negative-int>-11</negative-int>\n" + " <positive-overflow>18446744073709551616</positive-overflow>\n" + " <negative-overflow>-9223372036854775809</negative-overflow>\n" + " <hex>0x1234</hex>\n" + " <positive-float>12.5</positive-float>\n" + " <negative-float>-12.5</negative-float>\n" + " <attr empty=\"\"\n" + " text=\"123foo\"\n" + " positive-int=\"11\"\n" + " negative-int=\"-11\"\n" + " positive-overflow=\"18446744073709551616\"\n" + " negative-overflow=\"-9223372036854775809\"\n" + " hex=\"0x1234\"\n" + " positive-float=\"12.5\"\n" + " negative-float=\"-12.5\"\n" + " />\n" + "</test>\n"; + + XMLDocument doc; + ASSERT_TRUE(doc.ParseMemory(test_xml.data(), test_xml.size())); + + XMLNode root = doc.GetRootElement(); + ASSERT_TRUE(root.IsValid()); + + ASSERT_GET("empty", false, 66, false, 66.0); + ASSERT_GET("text", false, 66, false, 66.0); + ASSERT_GET("positive-int", true, 11, true, 11.0); + ASSERT_GET("negative-int", false, 66, true, -11.0); + ASSERT_GET("positive-overflow", false, 66, true, 18446744073709551616.0); + ASSERT_GET("negative-overflow", false, 66, true, -9223372036854775809.0); + ASSERT_GET("hex", true, 0x1234, true, 4660.0); + ASSERT_GET("positive-float", false, 66, true, 12.5); + ASSERT_GET("negative-float", false, 66, true, -12.5); +} + +#else // !LLDB_ENABLE_LIBXML2 + +TEST(XML, GracefulNoXML) { + std::string test_xml = + "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" + "<test>\n" + " <text attribute=\"123\">123</text>\n" + "</test>\n"; + + XMLDocument doc; + ASSERT_FALSE(doc.ParseMemory(test_xml.data(), test_xml.size())); + + XMLNode root = doc.GetRootElement(); + EXPECT_FALSE(root.IsValid()); + + XMLNode node = root.FindFirstChildElementWithName("text"); + EXPECT_FALSE(node.IsValid()); + + uint64_t uint_val; + EXPECT_FALSE(node.GetElementTextAsUnsigned(uint_val, 66, 0)); + EXPECT_EQ(uint_val, 66U); + EXPECT_FALSE(node.GetAttributeValueAsUnsigned("attribute", uint_val, 66, 0)); + EXPECT_EQ(uint_val, 66U); + + double double_val; + EXPECT_FALSE(node.GetElementTextAsFloat(double_val, 66.0)); + EXPECT_EQ(double_val, 66.0); +} + +#endif // LLDB_ENABLE_LIBXML2 Index: lldb/unittests/Host/CMakeLists.txt =================================================================== --- lldb/unittests/Host/CMakeLists.txt +++ lldb/unittests/Host/CMakeLists.txt @@ -12,6 +12,7 @@ SocketAddressTest.cpp SocketTest.cpp SocketTestUtilities.cpp + XMLTest.cpp ) if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android") Index: lldb/source/Host/common/XML.cpp =================================================================== --- lldb/source/Host/common/XML.cpp +++ lldb/source/Host/common/XML.cpp @@ -153,14 +153,11 @@ bool XMLNode::GetAttributeValueAsUnsigned(const char *name, uint64_t &value, uint64_t fail_value, int base) const { -#if LLDB_ENABLE_LIBXML2 - llvm::StringRef str_value = GetAttributeValue(name, ""); -#else - llvm::StringRef str_value; -#endif - bool success = false; - value = StringConvert::ToUInt64(str_value.data(), fail_value, base, &success); - return success; + if (llvm::to_integer(GetAttributeValue(name, ""), value, base)) + return true; + + value = fail_value; + return false; } void XMLNode::ForEachChildNode(NodeCallback const &callback) const { @@ -302,33 +299,21 @@ bool XMLNode::GetElementTextAsUnsigned(uint64_t &value, uint64_t fail_value, int base) const { - bool success = false; -#if LLDB_ENABLE_LIBXML2 - if (IsValid()) { - std::string text; - if (GetElementText(text)) - value = StringConvert::ToUInt64(text.c_str(), fail_value, base, &success); - } -#endif - if (!success) - value = fail_value; - return success; + std::string text; + if (GetElementText(text) && llvm::to_integer(text,value, base)) + return true; + + value = fail_value; + return false; } bool XMLNode::GetElementTextAsFloat(double &value, double fail_value) const { - bool success = false; -#if LLDB_ENABLE_LIBXML2 - if (IsValid()) { - std::string text; - if (GetElementText(text)) { - value = atof(text.c_str()); - success = true; - } - } -#endif - if (!success) - value = fail_value; - return success; + std::string text; + if (GetElementText(text) && llvm::to_float(text, value)) + return true; + + value = fail_value; + return false; } bool XMLNode::NameIs(const char *name) const {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
