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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:486
 
+#define PrintAuxMember(H, S, T, X)                                             \
+  W.print##H(S, T);                                                            \
----------------
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.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:491
+  W.print##H(S, T);                                                            \
+  if ((X = X - sizeof(T)) == 0)                                                \
+  return
----------------
DiggerLin wrote:
> hubert.reinterpretcast wrote:
> > This strikes me as extremely hazardous. What if we get a length value that is reflective of a partial field?
> thanks
We still have to build with C++14 compilers for the time being. Assigning a large 64-bit value to a 32-bit signed type is verboten. In any case, checking the table size against the last field of the table I described above would avoid this issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82549