[PATCH] D62957: [yaml2obj/obj2yaml] - Make RawContentSection::Content and RawContentSection::Size optional

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:403
   bool IsStatic = STType == SymtabType::Static;
+  const auto &Symbols = IsStatic ? Doc.Symbols : Doc.DynamicSymbols;
+
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > You probably need some test cases for DynamicSymbols as well as static Symbols.
> > OK, I'll add.
> > 
> > Do you think it worth to add tests for other 2 string sections (`.shstrtab`, `.dynstr`) too?
> > 
> I was wondering this. Those use the same code paths as .strtab, I believe, and there is no differences in behaviour, I think? I don't think it hurts to explicitly test them though necessarily, but don't think it critical by any means.
> I was wondering this. Those use the same code paths as .strtab, I believe, and there is no differences in behaviour, I think?

Yes, the path is the same and the difference should be negligible. Probably no need to duplicate the existing large test case I think.



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

https://reviews.llvm.org/D62957