[PATCH] D82549: [AIX][XCOFF] parsing xcoff object file auxiliary header

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added inline comments.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:489
+#define PrintAuxMember32(H, S, T)                                              \
+  if (offsetof(XCOFFAuxiliaryHeader32, T) + sizeof(AuxHeader->T) <= AuxiSize)  \
+  W.print##H(S, AuxHeader->T)
----------------
You can use `XCOFFAuxiliaryHeader32::T` in the `sizeof`.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:499
+  DictScope DS(W, "AuxiliaryHeader");
+  PrintAuxMember32(Hex, "Magic", AuxMagic);
+  PrintAuxMember32(Hex, "Version", Version);
----------------
Since the macro refers to it, move the macro definition into the scope of `AuxiSize`.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:527
+  PrintAuxMember32(Number, "Section number for .tdata", SecNumOfTData);
+  PrintAuxMember32(Number, "Section number for .tbss", SecNumOfTBSS);
+}
----------------
For symmetry, move the `undef` into the function body.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:486
 
+#define PrintAuxMember(H, S, T, X)                                             \
+  W.print##H(S, T);                                                            \
----------------
DiggerLin wrote:
> hubert.reinterpretcast wrote:
> > This macro does not operate within the confines of what a function can do with respect to its caller (it can cause the caller to return early). I do not believe that using a function-like naming style is appropriate. I also do not believe that using such a macro for control flow is desirable.
> > 
> > You can encode a table (yes, a macro is okay for that) with much the same information:
> > (format, description, pointer-to-member, offset in the table past-the-end of the member)
> > 
> > and use that table in the place where this macro is being invoked.
> for the function printNumber is a overload function.  using  a macro, the complie will determine which version of  printNumber  will be used when compile. if using a table, I think  how to make the code call the correct overload version of printNumber based in the parameter type when running may be complicated.
It can be solved using pointers-to-member in the table (I guess we don't really want to use those for selecting functions), but the macro is in better shape anyway now; thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82549