DavidSpickett created this revision.
Herald added a subscriber: mgorny.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The current handling of qHostInfo is a bunch of if-else,
one for each key we think we might find. This works fine,
but we end up repeating parsing calls and extending it
gets tricky.

To improve this I'm adding KeyValueExtractorGDBRemote.
This is a class that takes the packet response and splits it up
into the key-value pairs. Where a pair is "<key>:<value>;".

Then you can do Get<T>(<key>) to get an Optional<T>.
This Optional is empty if the key was missing or its value
didn't parse as the T you asked for.

Or you can GetWithDefault<T>(<key>, <default value>)
which always gives you a T but if search/parse fails
then you get <default value>.

Plus specific methods to get LazyBool and hex encoded
string keys.

This improves the packet handling code by:

- Putting the type, key name and default value on the same line.
- Reducing the number of repeated parsing calls. (fewer "!value.getAsInteger(0, 
...)" means fewer chances to forget a "!")
- Removing the need for num_keys_decoded, which was essentially a boolean but 
was treated as a running total. (replaced with HadValidKey)

We do pay an up front cost in building the map of keys up
front. However these packets are infrequent and the increase
in readability makes up for it.

This change adds the class and applies it to qHostInfo, keeping
the original logic intact. (future patches will apply it to
other packets and refactor post decode logic)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93225

Files:
  lldb/include/lldb/Utility/KeyValueExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/KeyValueExtractorGDBRemote.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/KeyValueExtractorGDBRemoteTest.cpp

Index: lldb/unittests/Utility/KeyValueExtractorGDBRemoteTest.cpp
===================================================================
--- /dev/null
+++ lldb/unittests/Utility/KeyValueExtractorGDBRemoteTest.cpp
@@ -0,0 +1,211 @@
+//===-- KeyValueExtractorGDBRemoteTest.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 "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/KeyValueExtractorGDBRemote.h"
+#include "llvm/Support/VersionTuple.h"
+
+TEST(KeyValueExtractorGDBRemoteTest, ParseNothing) {
+  StringExtractorGDBRemote response("");
+  KeyValueExtractorGDBRemote e(response);
+  ASSERT_FALSE(e.HadValidKey());
+  ASSERT_FALSE(e.Get<std::string>("test"));
+  ASSERT_FALSE(e.HadValidKey());
+}
+
+TEST(KeyValueExtractorGDBRemoteTest, ParseIncomplete) {
+  // Missing ending ';' so 'a' is not a valid key
+  StringExtractorGDBRemote response("a:abc");
+  KeyValueExtractorGDBRemote e(response);
+  ASSERT_FALSE(e.Get<std::string>("a"));
+}
+
+TEST(KeyValueExtractorGDBRemoteTest, GetWithDefaultHadValid) {
+  StringExtractorGDBRemote response("a:def;");
+  KeyValueExtractorGDBRemote e(response);
+
+  // HadValid not set if we returned the default value
+  uint32_t missing = e.GetWithDefault<uint32_t>("missing", 123);
+  ASSERT_EQ(missing, (uint32_t)123);
+  ASSERT_FALSE(e.HadValidKey());
+
+  // Not set if we found but failed to parse
+  uint32_t a_fails_parse = e.GetWithDefault<uint32_t>("a", 456);
+  ASSERT_EQ(a_fails_parse, (uint32_t)456);
+  ASSERT_FALSE(e.HadValidKey());
+
+  // Set if found and parsed, didn't use the default
+  std::string a_parsed = e.GetWithDefault<std::string>("a", "foo");
+  ASSERT_EQ(a_parsed, "def");
+  ASSERT_TRUE(e.HadValidKey());
+}
+
+TEST(KeyValueExtractorGDBRemoteTest, ParseUint32) {
+  StringExtractorGDBRemote response("valid:123;"
+                                    "invalid:abc;");
+  KeyValueExtractorGDBRemote e(response);
+
+  // Had valid is always false before any Get has been called
+  ASSERT_FALSE(e.HadValidKey());
+
+  ASSERT_FALSE(e.Get<uint32_t>("invalid"));
+  // Had valid is not set on parse failure
+  ASSERT_FALSE(e.HadValidKey());
+
+  llvm::Optional<uint32_t> valid = e.Get<uint32_t>("valid");
+  ASSERT_TRUE(valid);
+  ASSERT_EQ((uint32_t)123, *valid);
+
+  // Had valid now true because valid parsed successfully
+  ASSERT_TRUE(e.HadValidKey());
+
+  uint32_t get_default = e.GetWithDefault<uint32_t>("default", 99);
+  ASSERT_EQ((uint32_t)99, get_default);
+}
+
+TEST(KeyValueExtractorGDBRemoteTest, ParsePID) {
+  // This invokes the same path as uint32_t
+  // so this test just checks that we handle a wider
+  // type correctly.
+  StringExtractorGDBRemote response("invalid:???;"
+                                    "valid:0x1111222233334444;");
+  KeyValueExtractorGDBRemote e(response);
+
+  llvm::Optional<lldb::pid_t> invalid = e.Get<lldb::pid_t>("invalid");
+  ASSERT_FALSE(invalid);
+  ASSERT_FALSE(e.HadValidKey());
+
+  llvm::Optional<lldb::pid_t> valid = e.Get<lldb::pid_t>("valid");
+  ASSERT_TRUE(valid);
+  ASSERT_EQ((lldb::pid_t)0x1111222233334444, *valid);
+  ASSERT_TRUE(e.HadValidKey());
+}
+
+TEST(KeyValueExtractorGDBRemoteTest, ParseString) {
+  StringExtractorGDBRemote response("empty:;"
+                                    "valid:abc;"
+                                    "space:  ;");
+  KeyValueExtractorGDBRemote e(response);
+
+  // Empty strings are valid
+  llvm::Optional<std::string> empty = e.Get<std::string>("empty");
+  ASSERT_TRUE(empty);
+  ASSERT_EQ("", *empty);
+
+  ASSERT_TRUE(e.HadValidKey());
+
+  llvm::Optional<std::string> valid = e.Get<std::string>("valid");
+  ASSERT_TRUE(valid);
+  ASSERT_EQ("abc", *valid);
+
+  // All whitespace is valid
+  llvm::Optional<std::string> space = e.Get<std::string>("space");
+  ASSERT_TRUE(space);
+  ASSERT_EQ("  ", *space);
+
+  std::string get_default = e.GetWithDefault<std::string>("missing", "default");
+  ASSERT_EQ("default", get_default);
+}
+
+TEST(KeyValueExtractorGDBRemoteTest, ParseHexByteString) {
+  StringExtractorGDBRemote response("invalid:nothex;"
+                                    "invalid_end:68656cnothex;"
+                                    "valid:68656c6c6f;");
+  KeyValueExtractorGDBRemote e(response);
+
+  // Doesn't parse at all
+  llvm::Optional<std::string> invalid = e.GetHexByteString("invalid");
+  ASSERT_FALSE(invalid);
+  ASSERT_FALSE(e.HadValidKey());
+
+  // Decoding >0 chars is a success
+  llvm::Optional<std::string> invalid_end = e.GetHexByteString("invalid_end");
+  ASSERT_TRUE(invalid_end);
+  ASSERT_EQ("hel", *invalid_end);
+  ASSERT_TRUE(e.HadValidKey());
+
+  llvm::Optional<std::string> valid = e.GetHexByteString("valid");
+  ASSERT_TRUE(valid);
+  ASSERT_EQ("hello", *valid);
+
+  // Had valid is sticky so remains if a parse fails later
+  invalid = e.GetHexByteString("invalid");
+  ASSERT_TRUE(e.HadValidKey());
+}
+
+TEST(KeyValueExtractorGDBRemoteTest, ParseByteOrder) {
+  StringExtractorGDBRemote response("invalid:giant;"
+                                    "little:little;"
+                                    "big:big;"
+                                    "pdp:pdp;");
+  KeyValueExtractorGDBRemote e(response);
+
+  // We never return eByteOrderInvalid, just an empty optional
+  llvm::Optional<lldb::ByteOrder> invalid = e.Get<lldb::ByteOrder>("invalid");
+  ASSERT_FALSE(invalid);
+  ASSERT_FALSE(e.HadValidKey());
+
+  llvm::Optional<lldb::ByteOrder> little = e.Get<lldb::ByteOrder>("little");
+  ASSERT_TRUE(little);
+  ASSERT_EQ(lldb::eByteOrderLittle, *little);
+
+  ASSERT_TRUE(e.HadValidKey());
+
+  llvm::Optional<lldb::ByteOrder> big = e.Get<lldb::ByteOrder>("big");
+  ASSERT_TRUE(big);
+  ASSERT_EQ(lldb::eByteOrderBig, *big);
+
+  llvm::Optional<lldb::ByteOrder> pdp = e.Get<lldb::ByteOrder>("pdp");
+  ASSERT_TRUE(pdp);
+  ASSERT_EQ(lldb::eByteOrderPDP, *pdp);
+}
+
+TEST(KeyValueExtractorGDBRemoteTest, ParseVersionTuple) {
+  StringExtractorGDBRemote response("invalid:abc;"
+                                    "valid:1.2.3.4;");
+  KeyValueExtractorGDBRemote e(response);
+
+  // Usual parsing rules apply
+  llvm::Optional<llvm::VersionTuple> invalid =
+      e.Get<llvm::VersionTuple>("invalid");
+  ASSERT_FALSE(invalid);
+  ASSERT_FALSE(e.HadValidKey());
+
+  llvm::Optional<llvm::VersionTuple> valid = e.Get<llvm::VersionTuple>("valid");
+  ASSERT_TRUE(valid);
+  ASSERT_EQ(llvm::VersionTuple(1, 2, 3, 4), *valid);
+  ASSERT_TRUE(e.HadValidKey());
+}
+
+TEST(KeyValueExtractorGDBRemote, ParseLazyBoolKey) {
+  StringExtractorGDBRemote response("a_means_yes:a;"
+                                    "b_means_no:b;"
+                                    "c_is_invalid:c;");
+  KeyValueExtractorGDBRemote e(response);
+  std::string yes_value = "a";
+  std::string no_value = "b";
+
+  // We never return eLazyBoolCalculate, just an empty optional
+  llvm::Optional<lldb_private::LazyBool> c_is_invalid =
+      e.GetLazyBool("c_is_invalid", yes_value, no_value);
+  ASSERT_FALSE(c_is_invalid);
+  ASSERT_FALSE(e.HadValidKey());
+
+  llvm::Optional<lldb_private::LazyBool> a_means_yes =
+      e.GetLazyBool("a_means_yes", yes_value, no_value);
+  ASSERT_TRUE(a_means_yes);
+  ASSERT_EQ(lldb_private::eLazyBoolYes, *a_means_yes);
+  ASSERT_TRUE(e.HadValidKey());
+
+  llvm::Optional<lldb_private::LazyBool> b_means_no =
+      e.GetLazyBool("b_means_no", yes_value, no_value);
+  ASSERT_TRUE(b_means_no);
+  ASSERT_EQ(lldb_private::eLazyBoolNo, *b_means_no);
+}
Index: lldb/unittests/Utility/CMakeLists.txt
===================================================================
--- lldb/unittests/Utility/CMakeLists.txt
+++ lldb/unittests/Utility/CMakeLists.txt
@@ -11,6 +11,7 @@
   EventTest.cpp
   FileSpecTest.cpp
   FlagsTest.cpp
+  KeyValueExtractorGDBRemoteTest.cpp
   ListenerTest.cpp
   LogTest.cpp
   NameMatchesTest.cpp
Index: lldb/source/Utility/KeyValueExtractorGDBRemote.cpp
===================================================================
--- /dev/null
+++ lldb/source/Utility/KeyValueExtractorGDBRemote.cpp
@@ -0,0 +1,64 @@
+//===-- KeyValueExtractorGDBRemote.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/Utility/KeyValueExtractorGDBRemote.h"
+#include "lldb/Utility/StringExtractor.h"
+#include "llvm/ADT/StringSwitch.h"
+
+template <>
+llvm::Optional<std::string> KeyValueExtractorGDBRemote::Parse<
+    std::string, KeyValueExtractorGDBRemote::StringFormat::HexBytes>(
+    llvm::StringRef s) {
+  StringExtractor extractor(s);
+  std::string result;
+  // Anything > 0 chars decoded is a success
+  size_t got = extractor.GetHexByteString(result);
+  if (got)
+    return result;
+  return llvm::None;
+}
+
+KeyValueExtractorGDBRemote::KeyValueExtractorGDBRemote(
+    StringExtractorGDBRemote response)
+    : m_parsed_once(false), m_response(response) {
+  llvm::StringRef key;
+  llvm::StringRef value;
+  while (m_response.GetNameColonValue(key, value)) {
+    m_key_values[key] = value;
+  }
+}
+
+llvm::Optional<std::string>
+KeyValueExtractorGDBRemote::GetHexByteString(llvm::StringRef key) {
+  return GetImpl(key, Parse<std::string, StringFormat::HexBytes>);
+}
+
+llvm::Optional<lldb_private::LazyBool>
+KeyValueExtractorGDBRemote::GetLazyBool(llvm::StringRef key,
+                                        llvm::StringRef yes_string,
+                                        llvm::StringRef no_string) {
+  // We don't use GetKeyImpl here because it would set m_parsed_once
+  // before we have tried to convert to LazyBool.
+  auto key_value = m_key_values.find(key);
+  if (key_value == m_key_values.end())
+    return llvm::None;
+
+  // Note that m_parsed_once is not set until we parse the string into LazyBool
+
+  // We would StringSwitch here but that needs string literals
+  lldb_private::LazyBool decoded = lldb_private::eLazyBoolCalculate;
+  if (key_value->second == yes_string)
+    decoded = lldb_private::eLazyBoolYes;
+  else if (key_value->second == no_string)
+    decoded = lldb_private::eLazyBoolNo;
+  else
+    return llvm::None;
+
+  m_parsed_once = true;
+  return decoded;
+}
Index: lldb/source/Utility/CMakeLists.txt
===================================================================
--- lldb/source/Utility/CMakeLists.txt
+++ lldb/source/Utility/CMakeLists.txt
@@ -60,6 +60,7 @@
   StreamString.cpp
   StringExtractor.cpp
   StringExtractorGDBRemote.cpp
+  KeyValueExtractorGDBRemote.cpp
   StringLexer.cpp
   StringList.cpp
   StructuredData.cpp
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -31,6 +31,7 @@
 #include "ProcessGDBRemote.h"
 #include "ProcessGDBRemoteLog.h"
 #include "lldb/Host/Config.h"
+#include "lldb/Utility/KeyValueExtractorGDBRemote.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -1157,135 +1158,116 @@
     if (SendPacketAndWaitForResponse("qHostInfo", response, false) ==
         PacketResult::Success) {
       if (response.IsNormalResponse()) {
-        llvm::StringRef name;
-        llvm::StringRef value;
-        uint32_t cpu = LLDB_INVALID_CPUTYPE;
-        uint32_t sub = 0;
-        std::string arch_name;
-        std::string os_name;
-        std::string environment;
-        std::string vendor_name;
-        std::string triple;
-        std::string distribution_id;
-        uint32_t pointer_byte_size = 0;
-        ByteOrder byte_order = eByteOrderInvalid;
-        uint32_t num_keys_decoded = 0;
-        while (response.GetNameColonValue(name, value)) {
-          if (name.equals("cputype")) {
-            // exception type in big endian hex
-            if (!value.getAsInteger(0, cpu))
-              ++num_keys_decoded;
-          } else if (name.equals("cpusubtype")) {
-            // exception count in big endian hex
-            if (!value.getAsInteger(0, sub))
-              ++num_keys_decoded;
-          } else if (name.equals("arch")) {
-            arch_name = std::string(value);
-            ++num_keys_decoded;
-          } else if (name.equals("triple")) {
-            StringExtractor extractor(value);
-            extractor.GetHexByteString(triple);
-            ++num_keys_decoded;
-          } else if (name.equals("distribution_id")) {
-            StringExtractor extractor(value);
-            extractor.GetHexByteString(distribution_id);
-            ++num_keys_decoded;
-          } else if (name.equals("os_build")) {
-            StringExtractor extractor(value);
-            extractor.GetHexByteString(m_os_build);
-            ++num_keys_decoded;
-          } else if (name.equals("hostname")) {
-            StringExtractor extractor(value);
-            extractor.GetHexByteString(m_hostname);
-            ++num_keys_decoded;
-          } else if (name.equals("os_kernel")) {
-            StringExtractor extractor(value);
-            extractor.GetHexByteString(m_os_kernel);
-            ++num_keys_decoded;
-          } else if (name.equals("ostype")) {
-            ParseOSType(value, os_name, environment);
-            ++num_keys_decoded;
-          } else if (name.equals("vendor")) {
-            vendor_name = std::string(value);
-            ++num_keys_decoded;
-          } else if (name.equals("endian")) {
-            byte_order = llvm::StringSwitch<lldb::ByteOrder>(value)
-                             .Case("little", eByteOrderLittle)
-                             .Case("big", eByteOrderBig)
-                             .Case("pdp", eByteOrderPDP)
-                             .Default(eByteOrderInvalid);
-            if (byte_order != eByteOrderInvalid)
-              ++num_keys_decoded;
-          } else if (name.equals("ptrsize")) {
-            if (!value.getAsInteger(0, pointer_byte_size))
-              ++num_keys_decoded;
-          } else if (name.equals("os_version") ||
-                     name.equals(
-                         "version")) // Older debugserver binaries used the
-                                     // "version" key instead of
-                                     // "os_version"...
-          {
-            if (!m_os_version.tryParse(value))
-              ++num_keys_decoded;
-          } else if (name.equals("maccatalyst_version")) {
-            if (!m_maccatalyst_version.tryParse(value))
-              ++num_keys_decoded;
-          } else if (name.equals("watchpoint_exceptions_received")) {
-            m_watchpoints_trigger_after_instruction =
-                llvm::StringSwitch<LazyBool>(value)
-                    .Case("before", eLazyBoolNo)
-                    .Case("after", eLazyBoolYes)
-                    .Default(eLazyBoolCalculate);
-            if (m_watchpoints_trigger_after_instruction != eLazyBoolCalculate)
-              ++num_keys_decoded;
-          } else if (name.equals("default_packet_timeout")) {
-            uint32_t timeout_seconds;
-            if (!value.getAsInteger(0, timeout_seconds)) {
-              m_default_packet_timeout = seconds(timeout_seconds);
-              SetPacketTimeout(m_default_packet_timeout);
-              ++num_keys_decoded;
-            }
-          }
+        KeyValueExtractorGDBRemote extractor(response);
+
+        llvm::Optional<uint32_t> cpu = extractor.Get<uint32_t>("cputype");
+        uint32_t cpu_subtype =
+            extractor.GetWithDefault<uint32_t>("cpusubtype", 0);
+        llvm::Optional<std::string> arch_name =
+            extractor.Get<std::string>("arch");
+        llvm::Optional<std::string> triple =
+            extractor.GetHexByteString("triple");
+        llvm::Optional<std::string> distribution_id =
+            extractor.GetHexByteString("distribution_id");
+        llvm::Optional<std::string> os_build =
+            extractor.GetHexByteString("os_build");
+        llvm::Optional<std::string> hostname =
+            extractor.GetHexByteString("hostname");
+        llvm::Optional<std::string> os_kernel =
+            extractor.GetHexByteString("os_kernel");
+        llvm::Optional<std::string> os_name_value =
+            extractor.Get<std::string>("ostype");
+        llvm::Optional<std::string> vendor_name =
+            extractor.Get<std::string>("vendor");
+        llvm::Optional<lldb::ByteOrder> byte_order =
+            extractor.Get<lldb::ByteOrder>("endian");
+        llvm::Optional<uint32_t> pointer_byte_size =
+            extractor.Get<uint32_t>("ptrsize");
+        llvm::Optional<llvm::VersionTuple> os_version =
+            extractor.Get<llvm::VersionTuple>("os_version");
+        llvm::Optional<llvm::VersionTuple> version =
+            extractor.Get<llvm::VersionTuple>("version");
+        llvm::Optional<llvm::VersionTuple> maccatalyst_version =
+            extractor.Get<llvm::VersionTuple>("maccatalyst_version");
+        llvm::Optional<lldb_private::LazyBool> watchpoints_trigger =
+            extractor.GetLazyBool("watchpoint_exceptions_received", "after",
+                                  "before");
+        llvm::Optional<uint32_t> default_packet_timeout =
+            extractor.Get<uint32_t>("default_packet_timeout");
+
+        if (extractor.HadValidKey())
+          m_qHostInfo_is_valid = eLazyBoolYes;
+
+        if (os_build)
+          m_os_build = *os_build;
+        if (hostname)
+          m_hostname = *hostname;
+        if (os_kernel)
+          m_os_kernel = *os_kernel;
+
+        // Older debugserver binaries used the "version" key instead of
+        // "os_version"
+        if (os_version)
+          m_os_version = *os_version;
+        else if (version)
+          m_os_version = *version;
+
+        if (maccatalyst_version)
+          m_maccatalyst_version = *maccatalyst_version;
+
+        if (watchpoints_trigger)
+          m_watchpoints_trigger_after_instruction = *watchpoints_trigger;
+        else
+          m_watchpoints_trigger_after_instruction = eLazyBoolCalculate;
+
+        if (default_packet_timeout) {
+          m_default_packet_timeout = seconds(*default_packet_timeout);
+          SetPacketTimeout(m_default_packet_timeout);
         }
 
-        if (num_keys_decoded > 0)
-          m_qHostInfo_is_valid = eLazyBoolYes;
+        std::string os_name, environment;
+        if (os_name_value) {
+          ParseOSType(*os_name_value, os_name, environment);
+        }
 
-        if (triple.empty()) {
-          if (arch_name.empty()) {
-            if (cpu != LLDB_INVALID_CPUTYPE) {
-              m_host_arch.SetArchitecture(eArchTypeMachO, cpu, sub);
+        if (!triple) {
+          if (!arch_name) {
+            if (cpu) {
+              m_host_arch.SetArchitecture(eArchTypeMachO, *cpu, cpu_subtype);
               if (pointer_byte_size) {
-                assert(pointer_byte_size == m_host_arch.GetAddressByteSize());
+                assert(*pointer_byte_size == m_host_arch.GetAddressByteSize());
               }
-              if (byte_order != eByteOrderInvalid) {
-                assert(byte_order == m_host_arch.GetByteOrder());
+              if (byte_order) {
+                assert(*byte_order == m_host_arch.GetByteOrder());
               }
 
-              if (!vendor_name.empty())
+              if (vendor_name)
                 m_host_arch.GetTriple().setVendorName(
-                    llvm::StringRef(vendor_name));
-              if (!os_name.empty())
+                    llvm::StringRef(*vendor_name));
+
+              if (os_name.size())
                 m_host_arch.GetTriple().setOSName(llvm::StringRef(os_name));
-              if (!environment.empty())
+              if (environment.size())
                 m_host_arch.GetTriple().setEnvironmentName(environment);
             }
           } else {
-            std::string triple;
-            triple += arch_name;
-            if (!vendor_name.empty() || !os_name.empty()) {
-              triple += '-';
-              if (vendor_name.empty())
-                triple += "unknown";
+            std::string triple_str;
+            if (arch_name)
+              triple_str += *arch_name;
+
+            if (vendor_name || os_name.size()) {
+              triple_str += '-';
+              if (!vendor_name)
+                triple_str += "unknown";
               else
-                triple += vendor_name;
-              triple += '-';
+                triple_str += *vendor_name;
+              triple_str += '-';
               if (os_name.empty())
-                triple += "unknown";
+                triple_str += "unknown";
               else
-                triple += os_name;
+                triple_str += os_name;
             }
-            m_host_arch.SetTriple(triple.c_str());
+            m_host_arch.SetTriple(triple_str.c_str());
 
             llvm::Triple &host_triple = m_host_arch.GetTriple();
             if (host_triple.getVendor() == llvm::Triple::Apple &&
@@ -1303,19 +1285,20 @@
               }
             }
             if (pointer_byte_size) {
-              assert(pointer_byte_size == m_host_arch.GetAddressByteSize());
+              assert(*pointer_byte_size == m_host_arch.GetAddressByteSize());
             }
-            if (byte_order != eByteOrderInvalid) {
-              assert(byte_order == m_host_arch.GetByteOrder());
+            if (byte_order) {
+              assert(*byte_order == m_host_arch.GetByteOrder());
             }
           }
         } else {
-          m_host_arch.SetTriple(triple.c_str());
+          m_host_arch.SetTriple(triple->c_str());
+
           if (pointer_byte_size) {
-            assert(pointer_byte_size == m_host_arch.GetAddressByteSize());
+            assert(*pointer_byte_size == m_host_arch.GetAddressByteSize());
           }
-          if (byte_order != eByteOrderInvalid) {
-            assert(byte_order == m_host_arch.GetByteOrder());
+          if (byte_order) {
+            assert(*byte_order == m_host_arch.GetByteOrder());
           }
 
           LLDB_LOGF(log,
@@ -1326,10 +1309,10 @@
                         ? m_host_arch.GetArchitectureName()
                         : "<null-arch-name>",
                     m_host_arch.GetTriple().getTriple().c_str(),
-                    triple.c_str());
+                    triple->c_str());
         }
-        if (!distribution_id.empty())
-          m_host_arch.SetDistributionId(distribution_id.c_str());
+        if (distribution_id)
+          m_host_arch.SetDistributionId(distribution_id->c_str());
       }
     }
   }
Index: lldb/include/lldb/Utility/KeyValueExtractorGDBRemote.h
===================================================================
--- /dev/null
+++ lldb/include/lldb/Utility/KeyValueExtractorGDBRemote.h
@@ -0,0 +1,158 @@
+//===-- KeyValueExtractorGDBRemote.h ----------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_KEYVALUEEXTRACTORGDBREMOTE_H
+#define LLDB_UTILITY_KEYVALUEEXTRACTORGDBREMOTE_H
+
+#include "lldb/Utility/StringExtractorGDBRemote.h"
+#include "lldb/lldb-private-enumerations.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/VersionTuple.h"
+#include <map>
+#include <string>
+
+/// This class is used to handle GDB protocol packet responses
+/// of the form: <key>:<value>;<key2>:<value2>;<...>
+/// (e.g. qHostInfo and qProcessInfo)
+
+class KeyValueExtractorGDBRemote {
+public:
+  /// Construct from a packet response. Splits into key value pairs
+  /// but does not attempt to parse them.
+  explicit KeyValueExtractorGDBRemote(StringExtractorGDBRemote response);
+
+  /// Try to lookup and parse the given key.
+  /// If the key is present and parses as the given T
+  /// (Parse<T> succeeds), you get a valid Optional<T>.
+  /// If the key is not present or does not parse, you
+  /// get an empty Optional<T>.
+  ///
+  /// Type specific notes:
+  /// For std::string we allow empty values.
+  /// So "abc:;" will return a valid Optional with value "".
+  ///
+  /// For lldb::ByteOrder we do not return eByteOrderInvalid,
+  /// just an empty Optional.
+  ///
+  /// To add support for a new T, define a Parse function for it.
+  template <typename T> llvm::Optional<T> Get(llvm::StringRef key) {
+    return GetImpl(key, Parse<T>);
+  }
+
+  /// Like Get but returns default_value if the key is missing or did
+  /// not parse. Returning default_value is not counted as having found
+  /// a valid key.
+  template <typename T> T GetWithDefault(llvm::StringRef key, T default_value) {
+    llvm::Optional<T> got = Get<T>(key);
+    return got ? *got : default_value;
+  }
+
+  /// Try to lookup a key whose value is a hex encoded string.
+  /// Returns an empty Optional if:
+  /// * the key does not exist
+  /// * the value is not a hex byte string
+  /// * the value is empty
+  ///
+  /// Returns a valid optional if the key exists and some part
+  /// of the beginning of the value parses as hex bytes.
+  /// Meaning "a:41zzz;" returns "A", the rest is ignored.
+  llvm::Optional<std::string> GetHexByteString(llvm::StringRef key);
+
+  /// Try to lookup a key whose value maps to a lazy bool.
+  /// Where yes_string maps to LazyBoolYes and no_string maps to LazyBoolNo.
+  /// Returns a valid Optional if we find either of those exact values.
+  /// Otherwise returns an empty optional. (does not return LazyBoolCalculate)
+  llvm::Optional<lldb_private::LazyBool> GetLazyBool(llvm::StringRef key,
+                                                     llvm::StringRef yes_string,
+                                                     llvm::StringRef no_string);
+
+  /// Returns true if at least one Get<...> method has found and parsed
+  /// a key.
+  /// For example if we have "a:abc;" and do Get<uint32_t>("a") this method
+  /// will return false. Since we found the key but it failed to parse.
+  /// If we had "a:1;" instead, it would return true.
+  /// This is sticky so remains true once at least one Get<...> succeeds.
+  bool HadValidKey() const { return m_parsed_once; }
+
+private:
+  /// Generic logic to attempt to parse and record valid keys.
+  /// Note that GetLazyBool does not use this.
+  template <typename T>
+  llvm::Optional<T> GetImpl(llvm::StringRef key,
+                            llvm::Optional<T> (*parser)(llvm::StringRef)) {
+    auto key_value = m_key_values.find(key);
+    if (key_value == m_key_values.end())
+      return llvm::None;
+
+    llvm::Optional<T> got = parser(key_value->second);
+    if (got)
+      m_parsed_once = true;
+
+    return got;
+  }
+
+  template <typename T>
+  static llvm::Optional<
+      typename std::enable_if_t<std::is_integral<T>::value, T>>
+  Parse(llvm::StringRef s) {
+    T result;
+    // getAsInteger returns false on success
+    if (!s.getAsInteger(/*auto radix*/ 0, result))
+      return result;
+    return llvm::None;
+  }
+
+  template <typename T>
+  static llvm::Optional<
+      typename std::enable_if_t<std::is_same<llvm::VersionTuple, T>::value, T>>
+  Parse(llvm::StringRef s) {
+    llvm::VersionTuple ver;
+    // tryParse returns false on success
+    if (!ver.tryParse(s))
+      return ver;
+    return llvm::None;
+  }
+
+  template <typename T>
+  static llvm::Optional<
+      typename std::enable_if_t<std::is_same<lldb::ByteOrder, T>::value, T>>
+  Parse(llvm::StringRef s) {
+    lldb::ByteOrder byte_order = llvm::StringSwitch<lldb::ByteOrder>(s)
+                                     .Case("little", lldb::eByteOrderLittle)
+                                     .Case("big", lldb::eByteOrderBig)
+                                     .Case("pdp", lldb::eByteOrderPDP)
+                                     .Default(lldb::eByteOrderInvalid);
+    if (byte_order == lldb::eByteOrderInvalid)
+      return llvm::None;
+    return byte_order;
+  }
+
+  template <typename T>
+  static llvm::Optional<
+      typename std::enable_if_t<std::is_same<std::string, T>::value, T>>
+  Parse(llvm::StringRef s) {
+    return s.str();
+  }
+
+  // There is only one string format but writing it this way lets
+  // us reuse GetImpl for HexByteStrings.
+  enum class StringFormat {
+    HexBytes,
+  };
+  template <typename T, StringFormat F>
+  static llvm::Optional<T> Parse(llvm::StringRef value);
+
+  // Whether at least one key was correctly parsed in response to a Get<...>Key
+  // call
+  bool m_parsed_once;
+  // Inital map of key value pairs as their original strings
+  std::map<llvm::StringRef, llvm::StringRef> m_key_values;
+  StringExtractorGDBRemote m_response;
+};
+
+#endif // LLDB_UTILITY_KEYVALUEEXTRACTORGDBREMOTE_H
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to