[PATCH] D48904: [llvm-objdump] Add --archive-headers (-a) option

[Paul Semel via Phabricator via llvm-commits]

paulsemel added a comment.

Alright..
The fact is that this error is often occurring on the first entry when we create the archive file with GNU ar (I don't know why, I have to admit it). So, if we throw an error when we encounter this, we are simply not printing any entry of those files at all... (but the other entries are ok !)
That's actually what GNU objdump is doing. It is just reporting the file as not parsable in the archive, and going to the next file.
I think this is a good behavior. What do you think ?



================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:2129-2130
+  if (!ModeOrErr) {
+    consumeError(ModeOrErr.takeError());
+    outs() << "ill-formed archive entry.\n";
+    return;
----------------
jhenderson wrote:
> Why not just use report_error to print the appropriate message?
> 
> I assume that testing this would be non-trivial?
No, the fact is that, sometimes, only the first entry is ill-formed, and we still want to print the remaining part of the archive. I first did an error_report, but this leads to a non-functional option.
I think we should let it like this.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:2191
+static void DumpObject(ObjectFile *o, const Archive *a = nullptr,
+                       const Archive::Child *C = nullptr) {
   StringRef ArchiveName = a != nullptr ? a->getFileName() : "";
----------------
jhenderson wrote:
> If you get a moment, could you either NFC change the variable names to all be capitalised. Either that, or 'C' should be 'c'
let's remain this lowercase for this patch, I will do another one to correct all the variables in the file.


Repository:
  rL LLVM

https://reviews.llvm.org/D48904