[PATCH] D149440: [yaml2obj] Add support for load config section data.

[Jacek Caban via Phabricator via llvm-commits]

jacek added inline comments.


================
Comment at: llvm/lib/ObjectYAML/COFFYAML.cpp:565-566
+  IO.mapOptional("Size", S.Size, support::ulittle32_t(sizeof(S)));
+  if (S.Size < sizeof(S.Size)) {
+    IO.setError("Size must be at least " + Twine(sizeof(S.Size)));
+    return;
----------------
jhenderson wrote:
> Are you sure that wants to be `sizeof(S.Size)` in these two lines? Would it be worth giving `S` a more descriptive name?
Yes, that's what I meant. Size is the first member of load config structs. Load config that's too small to even fit the size field itself wouldn't make much sense. I will add a comment to clarify that, but I can also remove the check if you prefer (the check is not required from yaml2obj's point of view, I added it for completeness, it has a chance to catch some user errors).


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

https://reviews.llvm.org/D149440