[PATCH] D61767: [llvm-elfabi] Emit ELF header and string table section

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:123
+    StrTab.Align = 0;
+    for (const auto &Sym : Stub.Symbols) {
+      StrTab.Content.add(Sym.Name);
----------------
Nit: don't use `{` and `}` when body has only a single statement.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:130
+    const OutputSection<ELFT> *LastSection = Sections.back();
+    // Now set the Index and put sections names into .shstrtab .
+    uint64_t Index = 1;
----------------
Nit: No space.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:148
+        Align = 1;
+      if (Prev == nullptr) {
+        Sec->Offset = alignTo(StartOffset, Align);
----------------
Nit: don't use `{` and `}` only around single arm (also in this case they're unnecessary, see the comment above).


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:99-100
+
+// default constructable. This class just wraps it for the purpose of adding a
+// default constructor.
+class ELFStringTableBuilder : public StringTableBuilder {
----------------
Consider this.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:122
+    *reinterpret_cast<T *>(Data) = Value;
+  }
+  void fillStrTabShdr(ContentSection<ELFStringTableBuilder, ELFT> &StrTab,
----------------
Nit: would it be possible to separate function bodies with empty lines? You could also consider using `clang-format` which should handle that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61767