[PATCH] D17882: [ELF] - unify the use of uint32_t instead of unsigned in Target class for relocations types

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 4 06:11:16 PST 2016


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 ?
Then I can do that for sure.


http://reviews.llvm.org/D17882





More information about the llvm-commits mailing list