[PATCH] D78627: [obj2yaml] - Zero initialize program headers. NFCI.
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 23 02:41:10 PDT 2020
jhenderson added inline comments.
================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:764
if (YamlPhdr.Offset) {
- if (!Fragments.empty() && *YamlPhdr.Offset > PhdrFileOffset)
+ if (!Fragments.empty() && *YamlPhdr.Offset > Fragments.front().Offset)
reportError("'Offset' for segment with index " + Twine(PhdrIdx) +
----------------
grimar wrote:
> grimar wrote:
> > grimar wrote:
> > > jhenderson wrote:
> > > > Maybe worth pulling `Fragments.front().Offset` into a variable so that you don't have to repeat its call three times?
> > > OK, will do.
> > Ah, no, sorry, I can't do it. Otherwise we need to return the following line:
> >
> > ```
> > uint64_t PhdrFileOffset = Fragments.empty() ? 0 : Fragments.front().Offset;
> > ```
> > i.e. need to check that `Fragments` is not empty again. This is probably a bit more complicated than inlining it.
> >
> I.e. it would just revert all changes done by this patch here. As an alternative I can abandon this patch and just add `zero(Phdr);` in D78628.
> What do you think?
It's fine. It was only a suggestion. Happy for it to go in as is.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78627/new/
https://reviews.llvm.org/D78627
More information about the llvm-commits
mailing list