[PATCH] D108825: [yaml2obj] Allow variable number of directories

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: llvm/lib/ObjectYAML/COFFEmitter.cpp:464
          CP.Obj.OptionalHeader->DataDirectories) {
+      if (NumDir++ >= CP.Obj.OptionalHeader->Header.NumberOfRvaAndSize)
+        break;
----------------
I find the fused postincrement here a bit possibly confusing - e.g. with `NumberOfRvaAndSize = 1`, after the loop you would have written one entry, but end up with `NumDir = 2`. By doing a separate increment after the conditional, the variable would stay better in sync with reality.


================
Comment at: llvm/lib/ObjectYAML/COFFEmitter.cpp:474-477
+    if (NumDir < CP.Obj.OptionalHeader->Header.NumberOfRvaAndSize) {
+      OS << zeros(uint32_t(0));
+      OS << zeros(uint32_t(0));
+    }
----------------
jhenderson wrote:
> Don't you need to repeat this multiple times potentially, in case the `NumberOfRvaAndSize` value is more than one bigger than the number of `DataDirectories`?
Yeah, making this a while loop should be straightforward and make things more robust.

I guess it'd be a bit unexpected (as `CP.Obj.OptionalHeader->DataDirectories` is a hardcoded size for all potential types) but if the caller sets a larger value in `NumberOfRvaAndSize` then we should add dummy padding.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108825