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

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
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);
----------------
grimar wrote:
> 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;
> ```
It might be easier to read, but I'm okay with either one. It's up to you.


http://reviews.llvm.org/D21556