[PATCH] D124865: [AIX] support read global symbol of big archive

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/Archive.cpp:1204-1205
+                           Twine::utohexstr(GlobSymOffset) + " and size " +
+                           Twine(sizeof(BigArMemHdrType)) + "(0x" +
+                           Twine::utohexstr(sizeof(BigArMemHdrType)) +
+                           ") goes past the end of file");
----------------
I don't think we need the size to be printed in both hex and decimal. I think just hex is sufficient, having thought about it.


================
Comment at: llvm/lib/Object/Archive.cpp:1224-1225
+                           Twine::utohexstr(GlobalSymTblContentOffset) +
+                           " and size " + Twine(Size) + "(0x" +
+                           Twine::utohexstr(Size) +
+                           ") goes past the end of file");
----------------
I don't think we need the size to be printed in both hex and decimal. I think just hex is sufficient, having thought about it.


================
Comment at: llvm/test/tools/llvm-ar/malform-global-symbol-table-bigarchive.test:16
+
+# CHECK:  error: unable to load '[[FILE]]': truncated or malformed archive (global symbol table header at offset 0x20e and size 114 goes past the end of file)
+# CHECK2: error: unable to load '[[FILE]]': truncated or malformed archive (global symbol table content at offset 0x280 and size 37 goes past the end of file)
----------------
DiggerLin wrote:
> jhenderson wrote:
> > @MaskRay, I'm in two minds about this, so would like another opinion: would you prefer to have sizes in hex or decimal? On the one hand, decimal is more "natural" when speaking of sizes typically. On the other hand, it's much easier to do maths if the two things are in the same base, so my instinct would be to say that the sizes need to be hex too.
> I can change to using both hex and decimal. 
> truncated or malformed archive (global symbol table header at offset 0x20e and size 114(0x72) goes past the end of file) 
> @jhenderson 
As noted above, I think we should just have the size in one or other format, and I think I'd pick hex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124865