Skip to content

[lldb][NFCI] Constrain EventDataBytes creation #79508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

bulbazord
Copy link
Member

There are 3 ways to create an EventDataBytes object: (const char *), (llvm::StringRef), and (const void *, size_t len). All of these cases can be handled under llvm::StringRef. Additionally, this allows us to remove the otherwise unused SetBytes, SwapBytes, and SetBytesFromCString methods.

There are 3 ways to create an EventDataBytes object: (const char *),
(llvm::StringRef), and (const void *, size_t len). All of these cases
can be handled under `llvm::StringRef`. Additionally, this allows us to
remove the otherwise unused `SetBytes`, `SwapBytes`, and
`SetBytesFromCString` methods.
@llvmbot
Copy link
Member

llvmbot commented Jan 25, 2024

@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)

Changes

There are 3 ways to create an EventDataBytes object: (const char *), (llvm::StringRef), and (const void *, size_t len). All of these cases can be handled under llvm::StringRef. Additionally, this allows us to remove the otherwise unused SetBytes, SwapBytes, and SetBytesFromCString methods.


Full diff: https://github.com/llvm/llvm-project/pull/79508.diff

4 Files Affected:

  • (modified) lldb/include/lldb/Utility/Event.h (-10)
  • (modified) lldb/source/API/SBEvent.cpp (+2-1)
  • (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+5-5)
  • (modified) lldb/source/Utility/Event.cpp (+1-29)
diff --git a/lldb/include/lldb/Utility/Event.h b/lldb/include/lldb/Utility/Event.h
index 3de0401191caa7f..461d711b8c3f2c4 100644
--- a/lldb/include/lldb/Utility/Event.h
+++ b/lldb/include/lldb/Utility/Event.h
@@ -60,12 +60,8 @@ class EventDataBytes : public EventData {
   // Constructors
   EventDataBytes();
 
-  EventDataBytes(const char *cstr);
-
   EventDataBytes(llvm::StringRef str);
 
-  EventDataBytes(const void *src, size_t src_len);
-
   ~EventDataBytes() override;
 
   // Member functions
@@ -77,12 +73,6 @@ class EventDataBytes : public EventData {
 
   size_t GetByteSize() const;
 
-  void SetBytes(const void *src, size_t src_len);
-
-  void SwapBytes(std::string &new_bytes);
-
-  void SetBytesFromCString(const char *cstr);
-
   // Static functions
   static const EventDataBytes *GetEventDataFromEvent(const Event *event_ptr);
 
diff --git a/lldb/source/API/SBEvent.cpp b/lldb/source/API/SBEvent.cpp
index f12df2939420d63..cc611449e25099a 100644
--- a/lldb/source/API/SBEvent.cpp
+++ b/lldb/source/API/SBEvent.cpp
@@ -24,7 +24,8 @@ using namespace lldb_private;
 SBEvent::SBEvent() { LLDB_INSTRUMENT_VA(this); }
 
 SBEvent::SBEvent(uint32_t event_type, const char *cstr, uint32_t cstr_len)
-    : m_event_sp(new Event(event_type, new EventDataBytes(cstr, cstr_len))),
+    : m_event_sp(new Event(
+          event_type, new EventDataBytes(llvm::StringRef(cstr, cstr_len)))),
       m_opaque_ptr(m_event_sp.get()) {
   LLDB_INSTRUMENT_VA(this, event_type, cstr, cstr_len);
 }
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index eb42b9eb6cb6a57..4a06027501a898b 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1089,7 +1089,8 @@ Status ProcessGDBRemote::DoAttachToProcessWithID(
       const int packet_len =
           ::snprintf(packet, sizeof(packet), "vAttach;%" PRIx64, attach_pid);
       SetID(attach_pid);
-      auto data_sp = std::make_shared<EventDataBytes>(packet, packet_len);
+      auto data_sp =
+          std::make_shared<EventDataBytes>(llvm::StringRef(packet, packet_len));
       m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp);
     } else
       SetExitStatus(-1, error.AsCString());
@@ -1127,8 +1128,7 @@ Status ProcessGDBRemote::DoAttachToProcessWithName(
                                endian::InlHostByteOrder(),
                                endian::InlHostByteOrder());
 
-      auto data_sp = std::make_shared<EventDataBytes>(packet.GetString().data(),
-                                                      packet.GetSize());
+      auto data_sp = std::make_shared<EventDataBytes>(packet.GetString());
       m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp);
 
     } else
@@ -1374,8 +1374,8 @@ Status ProcessGDBRemote::DoResume() {
         return error;
       }
 
-      auto data_sp = std::make_shared<EventDataBytes>(
-          continue_packet.GetString().data(), continue_packet.GetSize());
+      auto data_sp =
+          std::make_shared<EventDataBytes>(continue_packet.GetString());
       m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp);
 
       if (!listener_sp->GetEvent(event_sp, std::chrono::seconds(5))) {
diff --git a/lldb/source/Utility/Event.cpp b/lldb/source/Utility/Event.cpp
index cac118182c75da3..863167e56bce6f5 100644
--- a/lldb/source/Utility/Event.cpp
+++ b/lldb/source/Utility/Event.cpp
@@ -111,17 +111,7 @@ void EventData::Dump(Stream *s) const { s->PutCString("Generic Event Data"); }
 
 EventDataBytes::EventDataBytes() : m_bytes() {}
 
-EventDataBytes::EventDataBytes(const char *cstr) : m_bytes() {
-  SetBytesFromCString(cstr);
-}
-
-EventDataBytes::EventDataBytes(llvm::StringRef str) : m_bytes() {
-  SetBytes(str.data(), str.size());
-}
-
-EventDataBytes::EventDataBytes(const void *src, size_t src_len) : m_bytes() {
-  SetBytes(src, src_len);
-}
+EventDataBytes::EventDataBytes(llvm::StringRef str) : m_bytes(str.str()) {}
 
 EventDataBytes::~EventDataBytes() = default;
 
@@ -147,20 +137,6 @@ const void *EventDataBytes::GetBytes() const {
 
 size_t EventDataBytes::GetByteSize() const { return m_bytes.size(); }
 
-void EventDataBytes::SetBytes(const void *src, size_t src_len) {
-  if (src != nullptr && src_len > 0)
-    m_bytes.assign(static_cast<const char *>(src), src_len);
-  else
-    m_bytes.clear();
-}
-
-void EventDataBytes::SetBytesFromCString(const char *cstr) {
-  if (cstr != nullptr && cstr[0])
-    m_bytes.assign(cstr);
-  else
-    m_bytes.clear();
-}
-
 const void *EventDataBytes::GetBytesFromEvent(const Event *event_ptr) {
   const EventDataBytes *e = GetEventDataFromEvent(event_ptr);
   if (e != nullptr)
@@ -186,10 +162,6 @@ EventDataBytes::GetEventDataFromEvent(const Event *event_ptr) {
   return nullptr;
 }
 
-void EventDataBytes::SwapBytes(std::string &new_bytes) {
-  m_bytes.swap(new_bytes);
-}
-
 llvm::StringRef EventDataReceipt::GetFlavorString() {
   return "Process::ProcessEventData";
 }

@jimingham
Copy link
Collaborator

LGTM This code was originally written before we used StringRef's, so making that conversion complete is good.

@bulbazord bulbazord merged commit 176d07d into llvm:main Jan 26, 2024
@bulbazord bulbazord deleted the constrain-event-data-bytes branch January 26, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants