[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