[PATCH] D21556: [ELF] - Implemented version script hierarchies.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 22 03:12:20 PDT 2016


grimar marked an inline comment as done.

================
Comment at: ELF/OutputSections.cpp:1510
@@ +1509,3 @@
+
+  // Make previous definition name be correctly connected to this one.
+  Verdaux[-1].vda_next = sizeof(Elf_Verdaux);
----------------
ruiu wrote:
> I don't think this comment helps reader to understand this piece of code. Instead, we could avoid [-1] by doing this, but I'd think your code is easier to read. So I'd just do without comment.
> 
>   Verdaux->vda_name = StrTabOffset;
>   if (HasParent) {
>     Verdaux->vda_next = sizeof(Elf_Verdaux);
>     ++Verdaux;
>     Verdaux->vda_name = getVersionNameStrTabOffset(ParentName);
>     Verdaux->vda_next = 0;
>   } else {
>     Verdaux->vda_next = 0;
>   }
>   ++Verdaux;
> 
> One thing I would do is to not do early return in this function. I believe this is easier to read in this case.
btw, may be ? 

```
Verdaux->vda_name = StrTabOffset;
if (HasParent) {
  Verdaux->vda_next = sizeof(Elf_Verdaux);
  ++Verdaux;
  Verdaux->vda_name = getVersionNameStrTabOffset(ParentName);
} 

Verdaux->vda_next = 0;
++Verdaux;
```


http://reviews.llvm.org/D21556





More information about the llvm-commits mailing list