[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/include/lldb/Utility/DataBuffer.h:58
+  ///     if the object contains no bytes.
+  virtual const uint8_t *GetBytesImpl() const = 0;
+
----------------
Are you sure this should be public?


================
Comment at: lldb/include/lldb/Utility/DataBuffer.h:98-103
   /// Get a const pointer to the data.
   ///
   /// \return
   ///     A const pointer to the bytes owned by this object, or NULL
   ///     if the object contains no bytes.
+  const uint8_t *GetBytes() const { return GetBytesImpl(); }
----------------
Replace with `using DataBuffer::GetBytes` ?


================
Comment at: lldb/include/lldb/Utility/DataBuffer.h:109-111
   llvm::ArrayRef<uint8_t> GetData() const {
     return llvm::ArrayRef<uint8_t>(GetBytes(), GetByteSize());
   }
----------------
ditto


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:361
   // Update the data to contain the entire file if it doesn't already
   if (data_sp->GetByteSize() < length) {
+    data_sp = MapFileDataWritable(*file, length, file_offset);
----------------
I guess this should be done unconditionally now.


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:382
 ObjectFile *ObjectFileELF::CreateMemoryInstance(
     const lldb::ModuleSP &module_sp, DataBufferSP &data_sp,
     const lldb::ProcessSP &process_sp, lldb::addr_t header_addr) {
----------------
I am assuming this will always point to a writable kind of a data buffer. Could we change the prototype to reflect that?


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2624
           DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+          WritableDataBuffer *data_buffer =
+              static_cast<WritableDataBuffer *>(data_buffer_sp.get());
----------------
add `// ObjectFileELF creates a WritableDataBuffer in CreateInstance`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122856/new/

https://reviews.llvm.org/D122856