[PATCH] D36255: Integrate manifest merging library into LLD.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/COFF/DriverUtils.cpp:347-348
 
-static std::string readFile(StringRef Path) {
-  std::unique_ptr<MemoryBuffer> MB =
-      check(MemoryBuffer::getFile(Path), "could not open " + Path);
-  return MB->getBuffer();
+static void
+createManifestXmlWithInternalMt(std::unique_ptr<MemoryBuffer> &OutputBuffer,
+                                std::string &DefaultXml) {
----------------
It is more natural to return a value rather than assigning to a variable passed by reference.


================
Comment at: lld/COFF/DriverUtils.cpp:414
+  if (OutputBuffer)
+    return OutputBuffer->getBuffer();
+  // Using built-in library failed, possibly because libxml2 is not installed.
----------------
I'm a little confused. Is this safe?

I mean, you are returning a pointer to a buffer owned by an object, which is in turn managed by a unique_ptr. But the unique_ptr is destructed as soon as you return from this function. What am I missing?


https://reviews.llvm.org/D36255