[PATCH] D92902: [llvm-elfabi] Add flag to keep timestamp when output is the same

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 17 04:30:30 PST 2020


jhenderson added a comment.

FYI, my last day working this year is tomorrow. I won't be certain to be able to review anything now until the New Year.



================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:674
+  // Write Stub to memory first.
+  std::vector<uint8_t> Buf(Builder.getSize(), 0);
+  Builder.write(Buf.data());
----------------
No need for the `0` initialisation value - that's the default anyway for a `uint8_t` when the vector is constructed.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:690
+  }
   Expected<std::unique_ptr<FileOutputBuffer>> BufOrError =
       FileOutputBuffer::create(FilePath, Builder.getSize());
----------------
Nit: I'd add a blank line before this line.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:697
                                  "` for writing");
-
   // Write binary to file.
+  std::unique_ptr<FileOutputBuffer> FileBuf = std::move(*BufOrError);
----------------
I'd add a blank line before this comment.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:699
+  std::unique_ptr<FileOutputBuffer> FileBuf = std::move(*BufOrError);
+  memcpy(FileBuf->getBufferStart(), Buf.data(), Builder.getSize());
 
----------------
This seems like a safer suggestion, in case later changes cause the vector's size to change post building.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:701-704
+  if (Error E = FileBuf->commit())
     return E;
 
   return Error::success();
----------------
You could probably fold these together into a single line like `return FileBuf->commit();`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92902



More information about the llvm-commits mailing list