[PATCH] D105522: [llvm-readobj][XCOFF] Fix the error dumping for the first item of StringTable.

[Esme Yi via Phabricator via llvm-commits]

Esme added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ObjDumper.cpp:56
 
-void ObjDumper::printAsStringList(StringRef StringContent) {
+void ObjDumper::printAsStringList(StringRef StringContent, uint8_t BeginByte) {
   const uint8_t *StrContent = StringContent.bytes_begin();
----------------
shchenz wrote:
> Can we add a comment about why we need to set start at `StrContent + StringSizeFieldLength`? 
> 
> Also can we add a line at the beginning of the string table to show the size of the string table?
> Also can we add a line at the beginning of the string table to show the size of the string table?
Same as above, the function is not only used by dumping the string table. But yes, I can print the size at XCOFFDumper::printStringTable.


================
Comment at: llvm/tools/llvm-readobj/ObjDumper.h:113
 
-  void printAsStringList(StringRef StringContent);
+  void printAsStringList(StringRef StringContent, uint8_t BeginByte = 0);
 
----------------
shchenz wrote:
> Maybe a more meaningful name would be `StringSizeFieldLength`?
The function also called by ObjDumper::printSectionsAsString, which dumps the specified section(s), which doesn't have the field containing string content's length. So I think `StringSizeFieldLength` may be confused in that case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105522