[PATCH] D53690: [llvm-objdump] Don't Crash When Using `-a` on Non-Archives (PR39402)

Xing via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 29 01:01:16 PDT 2018


Higuoxing marked an inline comment as done.
Higuoxing added inline comments.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:2309
 
   if (ArchiveHeaders && !MachOOpt)
+    printArchiveChild(ArchiveName, c);
----------------
eush wrote:
> Higuoxing wrote:
> > eush wrote:
> > > It seems like you could just add `&& c` here and in the second overload and the signature of `printArchiveChild` could stay the same.
> > Thanks a lot, but I think checking `nullptr` inside `printArchiveChild` will be more readable ?
> I disagree. `DumpArchive`, `DumpObject`, as well as other functions in this file are never called on `nullptr`, the pattern is to check first, then call. Why should `printArchiveChild` work differently?
> 
> Also, from code-complexity point of view, this patch introduces one new conditional statement where there could be none.
Sorry, I misunderstand your opinion. I just want to make a safe `printArchiveName`, it seems that not a good idea to introduce a new conditional statement ... thanks a lot


https://reviews.llvm.org/D53690





More information about the llvm-commits mailing list