[lldb] [Host] Refactor XML converting getters
authorMichał Górny <mgorny@moritz.systems>
Fri, 24 Sep 2021 11:25:27 +0000 (13:25 +0200)
committerMichał Górny <mgorny@moritz.systems>
Mon, 27 Sep 2021 12:26:33 +0000 (14:26 +0200)
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

Differential Revision: https://reviews.llvm.org/D110410

lldb/source/Host/common/XML.cpp
lldb/unittests/Host/CMakeLists.txt
lldb/unittests/Host/XMLTest.cpp [new file with mode: 0644]

index c3225d3f443369276854bc375e7b92fddb09279b..7f6f74396d7acbb912cb6ef76b412cf725f2a0a4 100644 (file)
@@ -6,10 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include <stdlib.h> /* atof */
-
 #include "lldb/Host/Config.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/XML.h"
 
 using namespace lldb;
@@ -153,14 +150,8 @@ llvm::StringRef XMLNode::GetAttributeValue(const char *name,
 
 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;
+  value = fail_value;
+  return llvm::to_integer(GetAttributeValue(name, ""), value, base);
 }
 
 void XMLNode::ForEachChildNode(NodeCallback const &callback) const {
@@ -302,33 +293,17 @@ bool XMLNode::GetElementText(std::string &text) const {
 
 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;
+
+  value = fail_value;
+  return GetElementText(text) && llvm::to_integer(text, value, base);
 }
 
 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;
+
+  value = fail_value;
+  return GetElementText(text) && llvm::to_float(text, value);
 }
 
 bool XMLNode::NameIs(const char *name) const {
index 1cc0cb081e494d7644d04915d5868423859636e6..14c4fe4d0b8a6d517c38cd3defbdde19a6bc3785 100644 (file)
@@ -12,6 +12,7 @@ set (FILES
   SocketAddressTest.cpp
   SocketTest.cpp
   SocketTestUtilities.cpp
+  XMLTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
diff --git a/lldb/unittests/Host/XMLTest.cpp b/lldb/unittests/Host/XMLTest.cpp
new file mode 100644 (file)
index 0000000..a845612
--- /dev/null
@@ -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