[PATCH] D55839: [elfabi] Add support for writing ELF header for binary stubs

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-elfabi/ELFObjHandler.cpp:51
+  template<class T>
+  void add(uint64_t Offset, const T& Value) {
+    MaxLocation = std::max(Offset + sizeof(T), MaxLocation);
----------------
clang-format? (I think it should be `const T &Value`)


================
Comment at: llvm/tools/llvm-elfabi/ELFObjHandler.cpp:442
+  using Elf_Ehdr = typename ELFT::Ehdr;
+  CommandWriter out;
+
----------------
out -> Out


================
Comment at: llvm/tools/llvm-elfabi/llvm-elfabi.cpp:151-152
     if (TBEWriteError) {
       WithColor::error() << TBEWriteError << "\n";
       exit(1);
     }
----------------
jakehehrlich wrote:
> jhenderson wrote:
> > You might want to consider factoring these out into a single function that takes an `Error`.
> Do you remember what you meant by this? I'm starting this back up again.
The WithColor::error() and exit(1) are repeated in a couple of places. It might be nice if they were a simple function that takes an llvm::Errror and does not return, e.g:

```
void reportError(Error Err) {
      WithColor::error() << Err << "\n";
      exit(1);
}
```

 although I feel like there might also be other functions available to do the same thing.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55839