[PATCH] D64470: [Object/ELF] - Improve error reporting for notes.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: include/llvm/Object/ELF.h:205
+    if (Phdr.p_type != ELF::PT_NOTE)
+      llvm_unreachable("Phdr is not of type PT_NOTE");
+
----------------
grimar wrote:
> jhenderson wrote:
> > I'm not sure making this llvm_unreachable is correct: if you are using libObject as a library, you could pass in an invalid Phdr, which should report an error, not an assertion.
> Note that method name is `notes_begin`. I.e it expects to accept note `Phdr` and all
> code we have do this check now.
> 
> I think that It is obviously a error in the code to pass `Phdr` not of type `PT_NOTE` here.
> Showing such error to user is unuseful, code just should never do that first of all.
Hmmm... okay, I'm not really convinced, but I don't feel strongly about it.

Either way, this can be simplified to `assert(Phdr.p_type != ELF::PT_NOTE && "Phdr is not of type PT_NOTE")`.


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

https://reviews.llvm.org/D64470