[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't think we actually disagree here. I'm aware of your use case (let's call it "dynamic mode") for appending to a buffer. I agree it's useful and I don't want to change that. What I want to remove is the other mode (non-dynamic). Presently, this is the only mode, but we're not actually making a good use of it. All of the existing use cases can be implemented with a "dynamic" buffer. And the code would be much simpler since there is only one mode to support -- one in which the data encoder owns the buffer it is writing to and can resize it at will.



================
Comment at: lldb/source/Expression/DWARFExpression.cpp:465
       std::unique_ptr<DataBufferHeap> head_data_up(
           new DataBufferHeap(m_data.GetDataStart(), m_data.GetByteSize()));
 
----------------
clayborg wrote:
> That would crash the program. The data here is coming from the DWARF from a mmap'ed read only file. We can't modify the data, and thus this is why we make a copy in the first place: so we can modify the DWARF data and fixup the address info.
With the current implementation yes. But this was meant to be done in conjuction with the changes to the DataEncoder constructor. The idea is that the constructor itself would copy the data into the owned buffer such that it can be freely modified or appended to. (In here we wouldn't make use of the append functionality, but that doesn't matter.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073