[PATCH] D93362: [llvm-elfabi] Support ELF file that lacks .gnu.hash section

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/include/llvm/Object/ELF.h:586
+  }
+  if (!SectionsOrError->size()) {
+    // Section headers are available but .dynsym header is not found.
----------------
jhenderson wrote:
> This looks inverted to me?
> 
> Also add a blank line after each of the if statemetns and for loops, to better breka this up into logical chunks.
You can use `SectionsOrError->empty()` for such checks.


================
Comment at: llvm/include/llvm/Object/ELF.h:596
+    return DynTable.takeError();
+  }
+  llvm::Optional<uint64_t> ElfHash;
----------------
You don't need curly bracers around a single line.


================
Comment at: llvm/include/llvm/Object/ELF.h:599
+  llvm::Optional<uint64_t> ElfGnuHash;
+  for (auto &Entry : *DynTable) {
+    switch (Entry.d_tag) {
----------------
Don't use `auto`, because the type is probably not obvious?


================
Comment at: llvm/include/llvm/Object/ELF.h:608
+  }
+  if (ElfGnuHash.hasValue()) {
+    Expected<const uint8_t *> TablePtr = toMappedAddr(*ElfGnuHash);
----------------
Just `if (ElfGnuHash)`, the same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93362