[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

[Owen Shaw via Phabricator via lldb-commits]

owenpshaw added a comment.

I'm not envisioning that anything else needs to change to use begin/end or care it's there.  I guess the way I look at it, having ObjectFile::LoadInMemory do begin/end is basically the same as what you're saying about having it be more process aware.

If Process is going to introduce new concepts for ObjectFile to use either way, isn't a high level of abstraction (batched writes) preferable to ObjectFile needing to know the fine details of flash memory blocks or that flash is even used?  And doesn't keeping the heavy lifting in ProcessGDB make it reusable should another case come along?

Hope you don't mind the pushback.  I think we're looking at this from different angles, and I'm genuinely asking to help my understanding of your thinking so hopefully we can converge on the same view.



================
Comment at: include/lldb/Target/MemoryRegionInfo.h:109-110
   ConstString m_name;
+  OptionalBool m_flash;
+  lldb::offset_t m_blocksize;
 };
----------------
clayborg wrote:
> Could these two be combined into one entry? Could "m_blocksize" be "m_flash_block_size" and if the value is zero, then it isn't flash?
Sure, I can change that.  Almost went that way initially, but thought the two fields made it a little clearer, and allowed for an error condition when flash is true, but the blocksize is invalid (0).  Collapsing to one field means falling back to a regular memory write rather than an error.  Not sure if it's a big deal either way.


https://reviews.llvm.org/D42145