[Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available

[Zachary Turner via lldb-commits]

zturner added inline comments.

================
Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:88
@@ +87,3 @@
+  if (data_sp && ObjectFilePECOFF::MagicBytesMatch(data_sp)) {
+    std::auto_ptr<ObjectFilePECOFF> objfile_ap(
+        new ObjectFilePECOFF(module_sp, data_sp, process_sp, header_addr));
----------------
`auto_ptr` should never be used under any circumstances.    Prefer `std::unique_ptr` any time you want to use an `auto_ptr`.  

Also, can you do an early return here?

```
if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp))
  return nullptr;
```

To create the `unique_ptr`, the preferred syntax (similar for `shared_ptr`) is this:

```
auto objfile_ap = llvm::make_unique<ObjectFilePECOFF>(module_sp, data_sp, process_sp, header_addr);
```

================
Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:423
@@ +422,3 @@
+    DataBufferSP buffer_sp(m_file.ReadFileContents(offset, size));
+    data = DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
+  } else {
----------------
Early return here.

`return DataExtractor(buffer_sp, GetByteOrder(), getAddressByteSize());`

================
Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:427
@@ +426,3 @@
+    if (process_sp) {
+      std::unique_ptr<DataBufferHeap> data_ap(new DataBufferHeap(size, 0));
+      Error readmem_error;
----------------
See earlier comment about `make_unique`.  Not critical, but a good habit to get into.


https://reviews.llvm.org/D24284