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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 18 01:11:58 PST 2020


grimar added inline comments.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:723
+      InputData = std::unique_ptr<uint8_t[]>(
+          new uint8_t[FileReadBuffer->getBufferSize()]);
+      memcpy(InputData.get(), FileReadBuffer->getBufferStart(), InputSize);
----------------
nit: I think you should be able to write in the following way to avoid using `new` explicitly:

```
InputData = std::unique_ptr<uint8_t[]>(FileReadBuffer->getBufferSize());
```


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:671
+  if (FilePath == "-" || sys::fs::status(FilePath, Stat))
+    Stat.permissions(static_cast<sys::fs::perms>(0777));
   ELFStubBuilder<ELFT> Builder{Stub};
----------------
I think this needs a comment.


================
Comment at: llvm/tools/llvm-elfabi/llvm-elfabi.cpp:74
+    return YAMLErr;
+  OutStr.str();
+
----------------
You don't need this line I think?


================
Comment at: llvm/tools/llvm-elfabi/llvm-elfabi.cpp:84
+      if (FileReadBuffer->getBuffer() == TBEStr)
+        return Error::success();
+    }
----------------
I'd make it shorter. Something like:
```
  if (ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrError =
          MemoryBuffer::getFile(FilePath))
    if ((*BufOrError)->getBuffer() == TBEStr)
      return Error::success();
```


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