[PATCH] D17882: [ELF] - unify the use of uint32_t instead of unsigned in Target class for relocations types
Rafael EspĂndola via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 4 06:33:35 PST 2016
On 4 March 2016 at 09:11, George Rimar <grimar at accesssoftek.com> wrote:
> grimar added inline comments.
>
> ================
> Comment at: ELF/Target.cpp:885
> @@ -884,4 +884,3 @@
> // This function returns a number of relocations that need to be skipped.
> -unsigned X86_64TargetInfo::relaxTls(uint8_t *Loc, uint8_t *BufEnd,
> - uint32_t Type, uint64_t P, uint64_t SA,
> - const SymbolBody *S) const {
> +size_t X86_64TargetInfo::relaxTls(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
> + uint64_t P, uint64_t SA,
> ----------------
> rafael wrote:
>> return unsigned for now.
> it is used as:
>
> ```
> void InputSectionBase<ELFT>::relocate(uint8_t *Buf, uint8_t *BufEnd,
> RelIteratorRange<isRela> Rels) {
> typedef Elf_Rel_Impl<ELFT, isRela> RelType;
> size_t Num = Rels.end() - Rels.begin();
> for (size_t I = 0; I < Num; ++I) {
> ...
> I += Target->relaxTls(BufLoc, BufEnd, Type, AddrLoc, SymVA, Body);
> ```
>
> I believe size_t is expected here. Why do you want to change that ?
>
> ================
> Comment at: ELF/Target.h:72
> @@ +71,3 @@
> + // This function returns how many relocations should be skipped from proccesssing.
> + virtual size_t relaxTls(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
> + uint64_t P, uint64_t SA, const SymbolBody *S) const;
> ----------------
> rafael wrote:
>> I would leave this one as unsigned. Or at least do it in another patch.
> Do you mean that I can change that to size_t but just commit separatelly ?
I meant reviewed separately, but given the use, it LGTM too. Just
commit separately.
Thanks,
Rafael
More information about the llvm-commits
mailing list