[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