[PATCH] D64281: [Object] Create MutableELFObject Class for Doing Mutations on ELFObjectFiles [Part 1]
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 19 22:17:31 PDT 2019
MaskRay added inline comments.
================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:84
+ uint64_t Count = 0;
+ std::transform(OriginalValues.begin(), OriginalValues.end(),
+ std::back_inserter(Mappings), [&Count](const OrigType &) {
----------------
`llvm::transform(OriginalValues, ...)`
================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:113
+ NewType &makeMutable(uint64_t Index, Args &&... Arguments) {
+ assert(Index < Mappings.size() && "Out of bounds");
+ if (Mappings[Index].Type == MappingType::New)
----------------
I think such `Out of bounds` assertions are probably not very necessary. They will certainly segfault if the user does have out-of-bounds accesses.
================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:116
+ return NewValues[Mappings[Index]];
+ NewValues.emplace_back(Arguments...);
+ Mappings[Index] = MappingType(NewValues.size() - 1, MappingType::New);
----------------
```
Mappings[Index] = MappingType(NewValues.size(), MappingType::New);
NewValues.emplace_back(std::forward<Args>(Arguments)...);
```
================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:150
+
+ /// moveSectionNext must be overriden because the section_iterators in
+ /// MutableELFObject work differently than in ELFObjectFile. In this class,
----------------
typo: overridden
`the section_iterators in MutableELFObject` -> `MutableELFObject::section_begin` ? (Just name it explicitly)
================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:184
+ /// @code{.cpp}
+ /// for (const SectionRef &Sec : MutObj.sections())
+ /// if (!Sec.getAlignment()) {
----------------
`for (SectionRef Sec : MutObj.sections())`
SectionRef has just 2 words. It is usually passed by value.
================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:187
+ /// auto MutSecOrErr = MutObj.getMutableSection(Sec);
+ /// if (!MutSecOrErr)
+ /// handleError(MutSecOrErr.takeError());
----------------
```
if (auto MutSecOrErr = MutObj.getMutableSection(Sec))
then;
else
handleError(...);
```
================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:189
+ /// handleError(MutSecOrErr.takeError());
+ /// MutSecOrErr->Header.sh_addrallgin = 1;
+ /// }
----------------
typo: sh_addralign
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64281/new/
https://reviews.llvm.org/D64281
More information about the llvm-commits
mailing list