[PATCH] D149610: [llvm-objdump][COFF] Skip empty entries when dumping the export table

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: lld/test/COFF/export32.test:83
 # CHECK7:      Ordinal      RVA  Name
-# CHECK7-NEXT:       1        0
+# CHECK7-NEXT:       1   0x1010  exportfn3
 # CHECK7-NEXT:       2   0x1010  foo
----------------
aganea wrote:
> mstorsjo wrote:
> > This change doesn't seem consistent and/or directly explainable by the change itself?
> The previous pattern was wrong. The RVA `0` was actually matching the first character of `0x1010`. 
Ok, that makes sense. Please mention and explain that bit in the commit message then!


================
Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:569
     } else {
-      outs() << format("    % 4d %# 8x", Ordinal, RVA);
+      outs() << format("   %5d %# 8x", Ordinal, RVA);
     }
----------------
aganea wrote:
> mstorsjo wrote:
> > This part of the change also seems a bit unrelated to the main change. If you want it bundled here, make sure to at least mention/explain it in the commit message.
> I was planning to commit it separately as a NFC patch, is that ok with you?
Yes, that sounds good, I’d rather have that as a separate commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149610