[Lldb-commits] [lldb] [LLDB][NFC] Move logging from GDBRemoteCommunication::CheckForPacket into a helper function (PR #201526)

Felipe de Azevedo Piovezan via lldb-commits lldb-commits at lists.llvm.org
Thu Jun 4 01:34:39 PDT 2026


https://github.com/felipepiovezan created https://github.com/llvm/llvm-project/pull/201526

This was a lot of the code in the middle of core logic.

>From 690e957101a02686c6e2c85904fb60e036521804 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Thu, 4 Jun 2026 09:09:46 +0100
Subject: [PATCH] [LLDB][NFC] Move logging from
 GDBRemoteCommunication::CheckForPacket into a helper function

This was a lot of the code in the middle of core logic.
---
 .../gdb-remote/GDBRemoteCommunication.cpp     | 122 ++++++++++--------
 1 file changed, 65 insertions(+), 57 deletions(-)

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index d73766501e758..42f45276a5c1a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -608,6 +608,67 @@ bool GDBRemoteCommunication::DecompressPacket() {
   return true;
 }
 
+// `payload` is the full raw packet (e.g. "$body#CC"); `content` is the body
+// between '$' and '#'.
+static void AddToLog(llvm::StringRef content, llvm::StringRef payload,
+                     uint64_t original_packet_size,
+                     GDBRemoteCommunicationHistory &history,
+                     bool compression_enabled) {
+  Log *log = GetLog(GDBRLog::Packets);
+  if (!log)
+    return;
+
+  // If logging was just enabled, flush the history. m_history has a flag
+  // ensuring this is done only once.
+  if (!history.DidDumpToLog())
+    history.Dump(log);
+
+  bool binary = false;
+  // Detect binary for packets starting with a '$' and with a '#CC' checksum.
+  if (payload.front() == '$' && payload.size() > 4)
+    for (char c : payload)
+      if (!llvm::isPrint(c) && !llvm::isSpace(c)) {
+        binary = true;
+        break;
+      }
+
+  uint64_t total_length = payload.size();
+  if (!binary) {
+    if (compression_enabled)
+      LLDB_LOGF(log, "<%4" PRIu64 ":%" PRIu64 "> read packet: %.*s",
+                original_packet_size, total_length, (int)(total_length),
+                payload.data());
+    else
+      LLDB_LOGF(log, "<%4" PRIu64 "> read packet: %.*s", total_length,
+                (int)(total_length), payload.data());
+    return;
+  }
+
+  StreamString strm;
+  // Packet header...
+  if (compression_enabled)
+    strm.Printf("<%4" PRIu64 ":%" PRIu64 "> read packet: %c",
+                original_packet_size, total_length, payload[0]);
+  else
+    strm.Printf("<%4" PRIu64 "> read packet: %c", total_length, payload[0]);
+  for (size_t i = 0; i < content.size(); ++i) {
+    // Remove binary escaped bytes when displaying the packet...
+    const char ch = content[i];
+    if (ch == 0x7d) {
+      // 0x7d is the escape character.  The next character is to be
+      // XOR'd with 0x20.
+      const char escapee = content[++i] ^ 0x20;
+      strm.Printf("%2.2x", escapee);
+    } else {
+      strm.Printf("%2.2x", (uint8_t)ch);
+    }
+  }
+  // Packet footer...
+  strm.Printf("%c%c%c", payload[total_length - 3], payload[total_length - 2],
+              payload[total_length - 1]);
+  log->PutString(strm.GetString());
+}
+
 GDBRemoteCommunication::PacketType
 GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len,
                                        StringExtractorGDBRemote &packet) {
@@ -717,63 +778,9 @@ GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len,
       assert(content_length <= total_length);
       size_t content_end = content_start + content_length;
 
-      bool success = true;
-      if (log) {
-        // If logging was just enabled and we have history, then dump out what
-        // we have to the log so we get the historical context. The Dump() call
-        // that logs all of the packet will set a boolean so that we don't dump
-        // this more than once
-        if (!m_history.DidDumpToLog())
-          m_history.Dump(log);
-
-        bool binary = false;
-        // Only detect binary for packets that start with a '$' and have a
-        // '#CC' checksum
-        if (m_bytes[0] == '$' && total_length > 4) {
-          for (size_t i = 0; !binary && i < total_length; ++i) {
-            unsigned char c = m_bytes[i];
-            if (!llvm::isPrint(c) && !llvm::isSpace(c)) {
-              binary = true;
-            }
-          }
-        }
-        if (binary) {
-          StreamString strm;
-          // Packet header...
-          if (CompressionIsEnabled())
-            strm.Printf("<%4" PRIu64 ":%" PRIu64 "> read packet: %c",
-                        (uint64_t)original_packet_size, (uint64_t)total_length,
-                        m_bytes[0]);
-          else
-            strm.Printf("<%4" PRIu64 "> read packet: %c",
-                        (uint64_t)total_length, m_bytes[0]);
-          for (size_t i = content_start; i < content_end; ++i) {
-            // Remove binary escaped bytes when displaying the packet...
-            const char ch = m_bytes[i];
-            if (ch == 0x7d) {
-              // 0x7d is the escape character.  The next character is to be
-              // XOR'd with 0x20.
-              const char escapee = m_bytes[++i] ^ 0x20;
-              strm.Printf("%2.2x", escapee);
-            } else {
-              strm.Printf("%2.2x", (uint8_t)ch);
-            }
-          }
-          // Packet footer...
-          strm.Printf("%c%c%c", m_bytes[total_length - 3],
-                      m_bytes[total_length - 2], m_bytes[total_length - 1]);
-          log->PutString(strm.GetString());
-        } else {
-          if (CompressionIsEnabled())
-            LLDB_LOGF(log, "<%4" PRIu64 ":%" PRIu64 "> read packet: %.*s",
-                      (uint64_t)original_packet_size, (uint64_t)total_length,
-                      (int)(total_length), m_bytes.c_str());
-          else
-            LLDB_LOGF(log, "<%4" PRIu64 "> read packet: %.*s",
-                      (uint64_t)total_length, (int)(total_length),
-                      m_bytes.c_str());
-        }
-      }
+      AddToLog(llvm::StringRef(m_bytes).slice(content_start, content_end),
+               llvm::StringRef(m_bytes).take_front(total_length),
+               original_packet_size, m_history, CompressionIsEnabled());
 
       m_history.AddPacket(m_bytes, total_length,
                           GDBRemotePacket::ePacketTypeRecv, total_length);
@@ -789,6 +796,7 @@ GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len,
       }
       packet = StringExtractorGDBRemote(*maybe_packet_str);
 
+      bool success = true;
       if (m_bytes[0] == '$' || m_bytes[0] == '%') {
         assert(checksum_idx < m_bytes.size());
         if (::isxdigit(m_bytes[checksum_idx + 0]) ||



More information about the lldb-commits mailing list